For those who've not been following along in the "patch manifest" thread, which eventually mutated into the "online file editing" thread, we now have a feature in a branch which allows editing of individual files via the web interface. This feature represents two completely new features in the web UI:

• The ability to edit a file and save changes. Doing so creates a commit which updates only that file.

• With my very tiny apologies to anti-JavaScripters: this feature works like a fully interactive single-page app, loading once and then performing all work via JavaScript. This allows, e.g., requesting a preview without losing one's place in a larger document (something not currently possible with the wiki or forum editors, but might, just might, be adapted to those apps in the near future).

i have set up a test/dummy repo which is a copy of one of my "real" repos, and have created 3 test-user accounts for people who are willing to help test this new app or simply want to try it out:

https://fossil.wanderinghorse.net/r/.cwal-dummy/fileedit (login info is below)

There's a link to that page on the home page (not the site menu) and it can be reached via certain other file-specific pages in order to open up the editor with a specific version of a given file.

It is still in development but it's essentially feature complete, awaiting only some refinements. (It's expected that the list of feature requests will grow once this gets some exposure, but the scope of the feature is not anticipated to grow much beyond what it currently demonstrates.) This feature will not be part of the upcoming 2.11 release - 2.12 is the earliest it would appear (which is still not 100% certain at this point).

Notes and caveats:

• The tester logins are named tester1, tester2, and tester3, and each one's password is its login name.

• This is a "throwaway" repo. There's nothing you can do it which will cause any permanent damage.

• The repo will be taken down if/when the feature is merged into trunk or if/when the feature is abandoned. (Or sooner if spambots manage to log in and cause Grief.)

• Which files are editable by this feature is defined by setting the fileedit-glob repo setting to a list of wildcards (a whitelist). In this particular repo that list is currently *.txt,*.md. (Sidebar: this feature is "primarily intended" for editing embedded docs, not code files. Anyone editing source code this way has only themselves to blame when they break their builds. Likewise, files with curious syntax requirements, e.g. hard tabs in Makefiles, will very likely be broken if edited this way. Also, beware of the potential for changes in the end-of-line style when editing online. See the EOL Policy selection list on the Commit tab.)

• Please provide feedback and bug reports via this thread or, if you prefer, contact me directly.

Thank you for your time, and Happy Hacking!

Just tried it very quick.

Looks very good so far.

There's 2 minor things I had time to notice during the 2 minutes I played with it.

• If I edit with Auto-refresh off, then go to Preview tab, turn Auto-refresh back ON doesn't refresh right away. I need to select another tab and come back to preview.

• On the commit tab, if the Message is empty and I press commit, it might be useful to have a feedback telling it won't commit with an empty message.

  • Perhaps it can use the yellow banner that you already use when no file is selected (only after pressing commit with empty message)

Also, do you have a plan about putting some "/fileedit" links with file and version taken from the current context on different existing pages ? Example: "/dir" or "/file" page ?

Or the manual file/version selection step will be mandatory from the "/fileedit" page ?

If I edit with Auto-refresh off, then go to Preview tab, turn Auto-refresh back ON doesn't refresh right away. I need to select another tab and come back to preview.

That's intentional. Don't overlook the refresh button next to the checkbox.

On the commit tab, if the Message is empty and I press commit, it might be useful to have a feedback telling it won't commit with an empty message.

Is that not exactly what it does? Note that it stops at the first error the server side detects, so it might say "error: file is not changed" before it detects that the comment is empty (i don't recall the order of the checks off-hand). All of that validation happens on the server after submitting the request - the UI does no validation of its own (to avoid logic duplication in C and JS).

Also, do you have a plan about putting some "/fileedit" links with file and version taken from the current context on different existing pages ? Example: "/dir" or "/file" page ?

There are already links in the contexts i've found where they make sense, but the usability problem with them is that the version number in most cases is not the leaf version. It's legal to edit a non-leaf version, but committing it requires creating a fork (also legal, but normally not intended, thus a checkbox on the commit page must be selected to allow a fork).

That's intentional. Don't overlook the refresh button next to the checkbox.

Yes I saw the refresh button. I just think that if I switch to auto-refresh, I expect the preview to start to auto-refresh immediately.

I expect the preview to start to auto-refresh immediately

It does... when the tab is switched to ;). Perhaps the button should be relabeled to "refresh the preview when visiting this tab," but that seems a bit verbose. The tooltip (admittedly not visible on mobile devices like this one) says something to that effect, though.

Very nice feature. Thanks. Some comments commited change.

Thanks. Some comments commited change

Syntax highlighting: unfortunately cannot work in the preview because highlighting is initialized at page-load time via the skin's integration of a third-party highlighter. The preview gets injected long after that step and fossil has no way of knowing that highlighting is desired or how to initiate it. That said: i am shaping the infrastructure to be able to eventually do that type of thing via event listeners registered by the skin.

Adding new files: the infrastructure supports it but it is highly unlikely to be added to the web interface because it's simply too easy to make a typo which causes a new file to be added with a differently-cased name than an existing file, especially if the repo (like most repos) has case-sensitive filenames. For example, you upload a file named a/b/c.txt and the repo already has A/b/c.Txt. Whether or not those are equivalent names depends on whether the repo is case-sensitive (which most are). Merging two branches with those files would cause Grief. There's simply too much room for unintended error there. It would also require the ability to upload a file, which i'm not keen on adding to the UI, but that's the lesser of the problems (and will eventually be needed in order to support adding new versions of binary files, such as images).

Diff refreshing automatically: requires that the UI be changed to use radio buttons so that it knows which diff mode to refresh to. i'm honestly not really keen on the idea of automatic diff refreshing because diffs are costly to calculate and a refresh is not normally needed. i don't really like auto-refresh of any tabs because it requires waiting on the server and repeating the same exact work over and over if the user keeps switching back and forth between tabs while they're looking around. Usability requires auto-refresh of the preview, but i'm not happy about that.

Did a fast test. Overall a reasonable experience. However...

On the commit tab, the checkboxes are styled similar to buttons. But only the checkbox itself is clickable. If you mark up the text labels for the checkboxes as real labels, it makes it easier to use the checkbox by increasing the clickable area.

So rather than:

 <input type="checkbox" name="dry_run" value="1" checked=""><span>Dry-run?</span>

something like:

 <label><input type="checkbox" name="dry_run" value="1" checked="">Dry-run?</label>

or maybe:

 <input type="checkbox" name="dry_run" value="1" checked=""><label for="dry_run">Dry-run?</label>

I think I have the syntax right, working from memory. I think a few other controls in the UI could use explicit labels as well rather than just text.

Have a great day.

-- rouilj

I think a few other controls in the UI could use explicit labels as well rather than just text.

i severely dislike label elements because their definition requires that each associated form element have an ID, which i consider to be a fundamental label design flaw (each ID implicitly introduces a new anchor in the doc which is accessible via adding #theID to the URL). The approach of embedding an element in the label tag is, AFAIK, a violation of the HTML standard, even if it's commonly supported for the sake of screen readers. (i may be misremembering that, though.) (Apropos: this UI makes no effort whatsoever to be "accessible.")

i realize that current approach is annoying to use, but am more likely to fix it with event listeners than label elements.

Edit: that said, most of those labeled checkboxes are generated via a common C routine and can have random IDs assigned to them in that code.

i realize that current approach is annoying to use, but am more likely to fix it with event listeners than label elements.

Edit: that said, most of those labeled checkboxes are generated via a common C routine and can have random IDs assigned to them in that code.

It turns out that all of those elements are generated via common C routines and none of them require an ID by the app, so it wasn't a problem to assign them random IDs for purposes of a label elements. That's been done and a new binary is building for my hoster as we speak:

https://www.fossil-scm.org/home/info/b46d11b871ef23d6

Hi Stephan:

The control embedded inside a label is supported in html5 and 4. Per:

https://www.w3.org/TR/2011/WD-html5-author-20110809/the-label-element.html

"The label represents a caption in a user interface. The caption can be associated with a specific form control, either using for attribute, or by putting the form control inside the label element itself."

I think you may be historically correct but html4 at least supported the embedded label as well. The html4 spec https://www.w3.org/TR/html401/interact/forms.html#h-17.9 says:

"To associate a label with another control implicitly, the control element must be within the contents of the LABEL element. In this case, the LABEL may only contain one control element. The label itself may be positioned before or after the associated control."

In these cases, you don't need the for= linking the label and control. But since you got it working that way, no sense in removing it.

-- rouilj

Stephan,

Thanks for the work, I'll do some testing on my own repo/... but there were a couple of UI/documentation suggestions, which I'm happy to pack up in a patch for you but I'd your thoughts first.

Commit or Save or Commit/Save

On the Commit page we have "Dry Run", "Allow Fork", ... buttons and the popup button help says "In Dry Run Mode the Save button ....". The town idiot (me) looks around and goes where is the "Save" button which is of course "Commit".

So perhaps the Commit page and button should be named Commit/Save, particularly for the general user which I guess is the target eventually.

Dry Run

Just wondering whether it would be disabled by default in normal operation or not.

File Content

Should there be a way to save a commit directly from the File Content Page; (I can just see my muppet managers spin locking on this one). Maybe the title change would fix that.

Thanks

Other file types and editors (aka download/upload)

The next user question I'll get is:

"I want to do edit project plan or image of a cat or use editor Z."

I understand this is out of scope but any observations or thoughts on what we should do?

Actually could we not do this with the current approach, and so allow editing of binary formats, or customised editors.

Commit or Save or Commit/Save

It was initially labeled Save, but that was as changed after feedback pointed out that it's performing a commit, so should be labeled that. i'll update the tooltip tomorrow. It's not supposed to be redefining any terminology, nor will attempt to "dumb it down" for those unfamiliar with SCM. Anyone using it should be fully aware of the side effects committing will cause.

Just wondering whether it would be disabled by default in normal operation or not.

There are two schools of thought on that and mine appears to have the minority membership: it's on by default to help prevent oopsies. That will likely change, by popular request, if this ever makes it to the trunk.

Should there be a way to save a commit directly from the File Content Page; (I can just see my muppet managers spin locking on this one). Maybe the title change would fix that.

It was initially on one page but was far, far too "busy", so was broken down into tabs, each dedicated to one specific feature. It's likely that at least one or two more commit options will eventually be added, which would make it even busier.

What title do you suggest? i'm not really happy with "file content" except that it's an accurate description.

Other file types and editors (aka download/upload)

The next user question I'll get is:

"I want to do edit project plan or image of a cat or use editor Z."

I understand this is out of scope but any observations or thoughts on what we should do?

It seems likely, but not guaranteed, that it will eventually be possible to upload new versions of binary files, but the chances of getting any editor embedded other than a textarea approach zero. The JS API is being shaped so that it will hypothetically be possible (via JS imported via skin customization) to plug in a fancy 3rd-party text (only) editor component to replace the textarea, but that's purely hypothetical at this point. Editing of non-text is as far out of scope as space flight system control is.

If you want to use editor Z or file type X, either create a checkout, like our ancestors taught us, or copy/paste between editor Z and the textarea element.

i personally have absolutely zero ambitions to implement any sort of non-text handling, but if the problem itches someone enough, they might implement it.

(Pardon brevity and typos - am working horizontally from a tablet with a dog in one arm.)

Firstly,

Thanks for working horizontally with a dog in one arm :-).

>Commit or Save or Commit/Save

It was initially labeled Save, but that was as changed after feedback pointed out that it's performing a commit, so should be labeled that. I'll update the tooltip tomorrow. It's not supposed to be redefining any terminology, nor will attempt to "dumb it down" for those unfamiliar with SCM. Anyone using it should be fully aware of the side effects committing will cause

Makes sense consistency with fossil languages.

Should there be a way to save a commit directly from the File Content Page; (I can just see my muppet managers spin locking on this one). Maybe the title change would fix that.

It was initially on one page but was far, far too "busy", so was broken down into tabs, each dedicated to one specific feature. It's likely that at least one or two more commit options will eventually be added, which would make it even busier.

What title do you suggest? i'm not really happy with "file content" except that it's an accurate description.

I think file content is fine, its just the commit tab and I can see you split them. One problem is that commit is a verb in this case, not a noun.Its do a commit, not what is committed. (2nd version of the Commit/Save question). I'll have a play and get back to you.

Other file types and editors (aka download/upload)

The next user question I'll get is:

"I want to do edit project plan or image of a cat or use editor Z."

I understand this is out of scope but any observations or thoughts on what we should do?

It seems likely, but not guaranteed, that it will eventually be possible to upload new versions of binary files,...

No web editing is all fine and we would need to replace the textarea, with the edited file or upload it or ...

Thanks again, I'll have a play and help out if possible.

Thanks for working horizontally with a dog in one arm :-).

It's difficult to tell her no.

I'll have a play...

If you'll wait another half hour or so a new version will be online which replaces the "save" verbiage and adds some internal bits which, it is hoped, may be useful in eventually replacing the textarea dynamically at page-load time via skin-injected JS. One of these days i hope to demonstrate a proof of concept for that, to find out if the infrastructure intended for it really works.

Edit: https://fossil-scm.org/fossil/info/f1b2e509e770b4be is now online.

Regarding "commit": i actually prefer the term "checkin", but fossil calls the command "commit" (with an alias of ci (checkin) for historical muscle-memory compatibility with CVS/SVN), so that's the term the page uses.

Hi Stephan,

This is very cool! Great job. Do you think your preview code could be carried over to the forum so we can switch between edit and preview as done in your demo?

While it may be a technically complicated issue, I think the ability to create files (txt, md, etc) would be remarkable. Many folks often look for a TODO list/app/program. While this isn't exactly the scope of Fossil, having an online TODO list, which is version controlled and editable in a browser, is appealing. I don't think the feature would need to be extended to support file uploads or binary support - keep it simple.

Anyway, I don't mean to take away from the creativity you've already shown us with this branch. Thanks a million for the efforts! Nice, I just discovered you can also edit files on a different open branch. I was expecting to use the files menu and go to /fileedit, but I can just select the branch and change to it all in one place.

Where are the links to activate the fileedit feature?

Syntax highlighting: unfortunately cannot work in the preview because highlighting is initialized at page-load time via the skin's integration of a third-party highlighter. The preview gets injected long after that step and fossil has no way of knowing that highlighting is desired or how to initiate it. That said: i am shaping the infrastructure to be able to eventually do that type of thing via event listeners registered by the skin.

Achievement unlocked...

Assuming one has highlightjs loaded via their skin, adding code like this to the end-of-page script initialization via the skin will highlight the /fileedit preview when it's updated:

if(fossil && fossil.page){
  fossil.page.addEventListener(
    'fileedit-preview-updated',
    (e)=>e.detail.querySelectorAll('code[class^=language-]')
      .forEach((e)=>hljs.highlightBlock(e))
  );
}

That needs some work in order to allow the listener to tell whether the preview is actually for wiki/markdown/inlined HTML content, as opposed to plain text or an iframe-hosted HTML file, so the data currently being passed to the event might yet change, but the concept is now proven using highlightjs on my local installation.

Edit: the event data has been expanded to include the content's mimetype and the current preview mode, so that the listener can make an informed decision about whether/how to respond to the update:

if(fossil && fossil.page){
  fossil.page.addEventListener(
    'fileedit-preview-updated',
    (ev)=>{
     if(ev.detail.previewMode==='wiki'){
       ev.detail.element.querySelectorAll(
         'code[class^=language-]'
        ).forEach((e)=>hljs.highlightBlock(e));
     }
    }
  );
}

Edit: the online test repo has been updated with this capability. Try loading one of the s2/manual/*.md files - most of them make use of it.

This is very cool! Great job. Do you think your preview code could be carried over to the forum so we can switch between edit and preview as done in your demo?

Assuming /fileedit ever hits trunk, the infrastructure will be in place to do exactly that: preview mode in the forum and wiki editor will be my next two stops after merging that infrastructure into trunk, and it will take practically no time at all to add this to those two pages (edit: famous last words - the form submission step is going to cause me a bit of grief because modern-day forms apparently like to submit as soon as any button in the form is clicked, and one has to bend over backwards to keep them from doing so, but this feature works without submission of the form). Without that infrastructure in place, though, there's no point in hammering on the forum preview just yet.

While it may be a technically complicated issue, I think the ability to create files (txt, md, etc) would be remarkable.

The infrastructure is there to do it, and it works, but i am personally extremely hesitant to add it to the web interface because of case-sensitivity issues: it would be very easy to intend to update an existing file but instead add a new file with the same name but different case (which means a different name).

Adding new files via the web interface, if it's ever added at all, will be a "version 2" feature, after the main features go online. It would require new UI elements which i have not yet accounted for and, frankly, am not up for developing right now ;).

... having an online TODO list, which is version controlled and editable in a browser, is appealing.

Create it in a checkout and edit it online. Or use the wiki - those can be created online.

Thanks a million for the efforts! Nice, I just discovered you can also edit files on a different open branch. I was expecting to use the files menu and go to /fileedit, but I can just select the branch and change to it all in one place.

You can actually edit files anywhere in the hierarchy except for a closed leaf. The main commit command absolutely prohibits checkins against a closed leaf, with no flag to override that, so this variant of commit does the same. It can make changes via any non-leaf, but committing to a non-leaf requires clicking the "allow fork" checkbox, as committing such a change so will necessarily create a fork.

The /fileedit file selection list only lists open leaves because that's the only realistic option is has. If you browse around the site, though, e.g. an finfo page like:

https://fossil.wanderinghorse.net/r/.cwal-dummy/finfo?name=doc/highlightjs/README.md&m=tip

If you're logged in with write access (like the test users have), you'll see a tiny little "edit" link to the right of each of those list entries. Tapping that will take you to the editor with that version of the file.

That feature isn't as useful as it might initially sound, though, because the top-most listing is rarely actually the leaf version of any branch: it's only the leaf if that file was modified via that leaf checkin. Because of that, the file selector was added to /fileedit to make it easier to select the most recent leaf version of files. Since the file list is filtered by an admin-defined glob, it "probably won't" grow too long on most repos.

Where are the links to activate the fileedit feature?

Rather than explain that directly, i'm going to ask you to check out the /fileedit "help" tab. The top line of help ostensibly says how to activate it, but:

1. If the thing it says has not already been done then /fileedit will display a highly visible warning explaining what has to be done in order to activate it. (Note that visiting /fileedit at all requires checkin privileges.)

2. If the text there isn't clear, please suggest edits or edit it directly in src/fileedit.c.

The JS API is being shaped so that it will hypothetically be possible (via JS imported via skin customization) to plug in a fancy 3rd-party text (only) editor component to replace the textarea, but that's purely hypothetical at this point.

That feature was just tested locally, but only enough to prove that it works. Without a 3rd-party editor widget to actually try it with, the feature is still effectively hypothetical, but "should work" just fine. Instructions are in the new docs dedicated to /fileedit's more esoteric features:

https://fossil-scm.org/fossil/doc/fileedit-ajaxify/www/fileedit-page.md

The infrastructure is there to do it, and it works, but i am personally extremely hesitant to add it to the web interface because of case-sensitivity issues.

What about doing a check for same file name with different case and issue a warning before to accept it? Or could even be refused right away, I think it’s so rare we want 2 files with same name but different case in same directory, even when the file system support it.

But I understand we have to stop somewhere, I’m sure very soon someone will ask to be able to remove or rename/move a file from web ui.

What about doing a check for same file name with different case and issue a warning before to accept it? Or could even be refused right away, I think it’s so rare we want 2 files with same name but different case in same directory, even when the file system support it.

Sure, we could, but "where does it stop?" Such a feature would require back-and-forth confirmation we don't yet have to handle, which means more infrastructure for a feature i don't want to implement in the first place ;).

That said: it's not my code - anyone with commit access is free to extend it.

But I understand we have to stop somewhere, I’m sure very soon someone will ask to be able to remove or rename/move a file from web ui.

That's a certainty, but my answer will be "use a checkout for that." Some will also want the ability to merge edits (use a checkout) and create new branches (the /info page already offers that via the edit link, so use that). This feature has one small purpose (quick/small edits to embedded docs), and is not intended to even come close to replacing a checkout or other existing pages. i'm going to be excessively stubborn on that point.

... a test/dummy repo which is a copy of one of my "real" repos, and have created 3 test-user accounts for people who are willing to help test this new app or simply want to try it out

My thanks to those of you who've been playing with it and providing feedback.

Please don't neglect to try the more esoteric features like creating a fork when editing a non-leaf version of a file - that repo is there to act as a testbed, so don't be afraid to break it (and if you do, then by all means report it!). The easiest way to get a non-leaf version is probably Files (/dir) ==> directory s2 ==> directory manual ==> tap almost any doc, as most were not modified in the tip version ==> tap the filename at the top of the resulting page ==> see the tiny "edit" links next to each listing (only visible when logged in with checkin rights, which the test accounts have).

For example, pick any non-tip version (via the "edit" link on the right) of one of these files:

https://fossil.wanderinghorse.net/r/.cwal-dummy/finfo?name=s2/manual/type-intro.md&m=tip

https://fossil.wanderinghorse.net/r/.cwal-dummy/finfo?name=s2/manual/misc-memcap.md&m=tip

https://fossil.wanderinghorse.net/r/.cwal-dummy/finfo?name=s2/manual/tips-and-tricks.md&m=tip

When committing to those you will be told that it would fork, but forking is not enabled. Just tap the Allow Fork checkbox and try again.

Thank you all again - it's exceedingly helpful to have others run through this, and reassuring that you haven't managed to hose the repo ;).

Please don't neglect to try the more esoteric features like creating a fork when editing a non-leaf version of a file

Thank you to whomever's been doing that!

To whomever asked, in the checkin comments, about how to determine the line ending style:

https://fossil.wanderinghorse.net/screenshots/Screenshot_20200514-232437_Firefox.jpg

That's unfortunately a difficult question with no simple answer...

Executive summary: always use the "inherit EOL style" option unless you know for a fact that you want to force a specific EOL style on the new version.

The details...

When text is assigned to an HTML textarea element and then POSTed via a form the HTML standard specifies that the EOLs are converted to Windows style. The initial version of /fileedit used a form, so that was what happened every time it saved something (and that's the reason the selection list was added). The current approach accesses the file content via the textarea's "value" property, which normalizes it to Unix newlines. When the app saves the content via AJAX, like it currently does, it is thus normalized to Unix newlines. In our editor, it's not really feasible to know which EOL style the file initially had, and the textarea element may represent it however it likes. (We "could" figure out the EOL style on the server and send that info along with the file, but so far there's been no need to do so. That info has, so far, only been relevant when saving.)

See /forumpost/ab03183cbc for more details about how textarea treats EOLs and a link to the HTML standard on that topic.

So what happens when /fileedit commits, by default, is that fossil loads the previous version, figures out what EOL style it uses, and converts, if needed, the being-saved file to the same EOL style. If the user specifies a specific EOL style, that will be applied instead of guessing based on the previous version.

In very nearly 100% of cases, "inherit" is the correct option to select for EOL style. Testing which EOL style the input originally had is not currently possible in the web UI, but can be done locally with the CLI version of the command (but it's tedious and requires an editor which can show the EOLs (that feature wasn't much fun to develop)).

The other EOL options are provided for that 1 in 10000+ chance that a user wants to explicitly set the EOL style. They are also provided for that 1 in ... ... maybe 1000 ... chance that someone manages to convince me to expose the "add a new file" feature via the web UI (the internals support it, but adding it to the UI is something i will stubbornly resist (but cannot rule out that someone else may implement it)). When using that hypothetical feature, it may be necessary for a user to explicitly specify which EOL style to use for the new file. e.g. Unix shell scripts will not run if they're saved in Windows format because Unix systems treat the \r character as part of the interpreter's name, and then can't find an interpreter named /bin/sh\r (i've seen that mistake bring down a production stock market trading system).

To whomever asked, in the checkin comments, about how to determine the line ending style:

It's me, sorry I didn't have time to reply here yet.

Actually, I made different edit with different EOL to try different possibilities, I know that in real life I would always use "inherit" .

Moreover, I downloaded a tar of each of those checkin I've force a specific EOL to verify if it works and found that all of them was "Unix" line ending.

So it seems that when I force EOL to "Windows", it did nothing.

Moreover, I downloaded a tar of each of those checkin I've force a specific EOL to verify if it works

That's a good idea. You can also clone it and then pull after making an online edit. i will take a look at the EOL conversion the next time i'm back on the computer. Thank you for the testing and the report!

So it seems that when I force EOL to "Windows", it did nothing.

There were two unrelated problems, but the first one stopped you from seeing the second one:

1. Since /fileedit was ported to ajax, i neglected to pass on the EOL conversion flag to the server. Ooops.

2. If a file had Windows EOLs and was told to explicitly convert to Windows EOLs, i was passing it to blob_add_cr(), which (i didn't realize) unconditionally adds a \r before every \n, leading to \r\r\n (so BUILDING.txt has that (mis-)format in some commits). When told to force Windows newlines, if the input already has them, it now does nothing, rather than pass the input to blob_add_cr().

The new binary is on the hoster - be sure to reload the page for the ajax-side fix.

Thank you again for your support!

While testing EOL stuff again, I found another issue.

When editing a file that has same content from a previous version of the same file (when it shows the [more...] link), it commit against the checkin of the first version of that file with this specific content.

Example: BUILDING.txt from this checkin eeb323af7cb87771 have same content as from many checkin before.

To get the "/fileedit" link for this file:

• I go on the /info page of this checkin.
• I spot the: Modified BUILDING.txt from [dc27f666cb] to [4d18716bdb] and I press on the right artifact link: [4d18716bdb659e3e]
• From there, we can see: ... [Edit][More...]. If I Edit from there, it will not commit from the checkin of the info page I come from, but against the checkin corresponding to the first instance of this artifact for this file name (I suppose).

May be the [Edit] link should be on the /file page when accessed from the [/info] page of a particular checkin.

That way we are sure against which checkin we are about to commit.

From there, we can see: ... [Edit][More...]. If I Edit from there, it will not commit from the checkin of the info page I come from, but against the checkin corresponding to the first instance of this artifact for this file name (I suppose).

See /forumpost/10f867ef6d for why that happens. That aspect of fossil's file history was what prompted the addition of the file selector list in /fileedit - without that list it is troublesome to find the leaf version of a file, especially if the content reverts to an older state.

May be the [Edit] link should be on the /file page when accessed from the [/info] page of a particular checkin.

i'm not sure what you mean by that. Walk me through the clicks you're following/would like to follow. We can certainly add the edit link just about anywhere, so long as we have a filename and checkin version available at that point.

Would it be possible to have the title updated to reflect the file being edited? Throughout the entire process, it shows "File Editor" as the tab. Maybe it's not possible since the page isn't reloading, just the ajax thing is doing to work.

Not Martin, and I hope it doesn't mind, but maybe an edit link can go here as well...

Walk me through the clicks you're following/would like to follow. We can certainly add the edit link just about anywhere, so long as we have a filename and checkin version available at that point.

Could it be added to this page: https://fossil.wanderinghorse.net/r/.cwal-dummy/file?name=s2/manual/doc-maintenance.md&ci=cd2007f8fa747f61

1. Click files
2. Click file ages
3. Find a txt/md file
4. You can see the content of the file, but need to see the artifact to edit it.

Actually, you get to the same page when doing:

1. Click checkin hash from timeline
2. Click "file" link from the "/info" page header.
3. Click on the file you want
4. You can see the content of the file, but need to see the artifact to edit it.

A new issue, I'm not able to commit when selecting "Multi-Line" comment.

Sometimes, it just commit and silently strip all the lines except the first one.

Sometimes it just "Hang" and does nothing. (showing: <Date/Time>: Checking in ... just below the tabs)

Achievement Unlocked: Editor Widget Dynamically Replaced

This proof-of-concept screenshot demonstrates /fileedit's ability for a fossil skin to replace its plain textarea editing component with a 3rd-party editor widget:

https://fossil.wanderinghorse.net/screenshots/fossil-fileedit-replace-editor-CodeMirror.png

The JS seen on the bottom of the screenshot was entered into the JS console, but would "normally" be added via the site's skin. It replaces /fileedit's textarea with a CodeMirror embedded editor widget (completely devoid of customization, so it still looks just like a text area and isn't sized-to-fit properly).

i also tried the Quill embeddable editor but the copy i have dies in endless recursion in its constructor, long before it gets to anything having to do with /fileedit.

More details about the ability to swap out the editor widget and integrate 3rd-party syntax highlighting into the file preview can be found in The Docs:

https://fossil-scm.org/fossil/doc/fileedit-ajaxify/www/fileedit-page.md

Yeah, I've had this same problem. My commit is this:

Adding giberish
on
two
long
lines

With dry-run enabled, file editor reports: 2020-05-15 02:05:28 UTC: Empty checkin comment is not permitted.

Would it be possible to have the title updated to reflect the file being edited?

Sure, that's not a problem at all (one benefit of it being implemented in JS is that we can manipulate all of the page at will), with the minor caveat that the name can be arbitrarily long. i'll get that done in a little while.

Similarly, we can also update the URL bar, using HTML5's history support, with a permalink to the current file/version, but i have not yet implemented that.

Would it be possible to have the title updated to reflect the file being edited?

Something like:

https://fossil.wanderinghorse.net/screenshots/fossil-fileedit-set-page-title.png

?

i'm tempted to replace all directory parts of the name after the first with "...", so foo/bar/baz/barre.txt would become foo/.../barre.txt in the title. Opinions?

The whole implementation for that is:

 /**
    Sets the innerText of the page's TITLE tag to
    the given text and returns this object.
  */
 F.page.setPageTitle = function(title){
   const t = document.querySelector('title');
   if(t) t.innerText = title;
   return this;
 };

...

 this.setPageTitle("Edit: "+this.finfo.filename);

("F" is a file-local alias for windows.fossil.)

Could it be added to this page:

i avoided adding it to that page only because there are currently no "menu" entries on that page. It's not a problem to add it, though.

Something like:

Yes! So you select the file, click load file and it changes the title?

i'm tempted to replace all directory parts of the name after the first with "...", so foo/bar/baz/barre.txt would become foo/.../barre.txt in the title. Opinions?

Yeah, I think that's a good idea. Anyone else have an idea?

What if it were added to the same section as the artifact link? "edit file", "edit this file". That's personally where I would look for it.

What do you say, Martin?

Yeah, when I tried with Firefox, I got the same Error message as you <Date/Time>: Empty checkin comment is not permitted. (with a yellow background).

But with Safari, it only show: <Date/Time>: Checking in ...

Firefox... Safari...

i have not tried any browsers other than (mostly) Firefox and (a little bit) Chrome, and don't have non-Linux/Android platforms to test on. The code only uses "modern" JS and HTML5 features, but it is certainly possible that i've used a JS feature which is too new for Safari (that said, i try to avoid any features standardized after 2015-ish, and avoid anything not yet standardized). If you can submit any errors from the dev console (if it has such a thing?), that would be really helpful.

A new issue, I'm not able to commit when selecting "Multi-Line" comment.

Bug: i failed to update the post of the commit when adding the multi-line toggle. That'll be fixed in a bit.

I see this in firefox's console with multi-line messages:

Ajax error: 
load { target: XMLHttpRequest, isTrusted: true, lengthComputable: true, loaded: 51, total: 51, srcElement: XMLHttpRequest, currentTarget: XMLHttpRequest, eventPhase: 2, bubbles: false, cancelable: false, … }
fossil.fetch.js:9:9
    onerror https://fossil.wanderinghorse.net/r/.cwal-dummy/builtin/fossil.fetch.js?cache=9c196aa2:9
    onload https://fossil.wanderinghorse.net/r/.cwal-dummy/builtin/fossil.fetch.js?cache=9c196aa2:72

and

Error JSON: 
Object { error: "Empty checkin comment is not permitted." }
fossil.fetch.js:17:9

Does this help you?

i avoided adding it to that page only because there are currently no "menu" entries on that page. It's not a problem to add it, though.

There actually is a menu - the submenu - but my eyes were looking for the not-quite-menu list of actions like [info][history][...]. Now there's an Edit entry for editable user/file combinations.

i'm testing the fix for the multi-line comment and will then build/post a new binary.

I see this in firefox's console with multi-line messages:

That was a side effect of the multi-line comment bug, the fix for which is literally scp'ing to the hoster right this second. Done. Don't forget to reload the page for the latest JS code.

Thank you guys again for the assistance!

I get something similar on Safari.

[Error] Failed to load resource: the server responded with a status of 400 () (fileedit, line 0)
[Error] Ajax error:
  ...
  ...
arguments: TypeError: 'arguments', 'callee', and 'caller' cannot be accessed in this context.
caller: TypeError: 'arguments', 'callee', and 'caller' cannot be accessed in this context.
  ...
  ...
Event Prototype
	(anonymous function) (fossil.fetch.js:9)
	(anonymous function) (fossil.fetch.js:72)

Will see if latest Stephan commit fix this problem on both browser.

Works on Safari with your latest fix..

arguments: TypeError: 'arguments', 'callee', and 'caller' cannot be accessed in this context.

caller: TypeError: 'arguments', 'callee', and 'caller' cannot be accessed in this context.

Those are unexpected. The first can happen if arguments is used in a new-style function ((x)=>{...} or (x)=>expr), where it's not allowed.

i've just looked through all of the new JS code and see no such functions which reference arguments (only "old-style" funcs do).

callee and caller are not used anywhere in this code - JS deprecated callee years ago and offers a nicer solution for that (and caller i've never once used in 20-odd years).

i'm at a bit of a loss to explain that one, but suspect that it's being triggered by something the browser is plugging in. Chrome is notorious for spitting out warnings in the console for URLs and CSS features it injects itself, with no way to bypass them on the developer's part.

[stephan@st3v3:~/fossil/fossil/src]$ grep -w callee *.js
[stephan@st3v3:~/fossil/fossil/src]$ grep -w caller *.js
fossil.fetch.js:   with state other than what the caller provided.
fossil.fetch.js:   function's caller.

Could it be added to this page: https://fossil.wanderinghorse.net/r/.cwal-dummy/file?name=s2/manual/doc-maintenance.md&ci=cd2007f8fa747f61

That one works now, BTW. Check the submenu, noting that it only appears if you're logged in w/ checkin rights and that file is in the whitelist globs.

Yep, I saw and I think this is a perfect spot for it.

When diffing long lines, there doesn't appear to be a horizontal scroll bar:

https://imgur.com/wHStNRt

Do you think there should be one?

When diffing long lines, there doesn't appear to be a horizontal scroll bar:

Just coincidentally i noticed about 10 minutes ago that there's supposed to be a piece of JS code (sbsdiff.js) which gets emitted for SBS diffs but is not getting emitted, and that might explain what you're seeing.

That code is not designed to be able to be reused multiple times in the same page, so i need to adapt it to make it usable that way before i can try that out.

Quality of life improvement request for consideration...

While editing a document, the user may get distracted and click outside of the /fileedit, for instance the timeline, and lose their work. Perhaps there could be a notification/popup the work will be lost. This is a common thing I see when you're in the middle of a post and an attempt to discard it prompts for confirmation. I think github issues will actually keep the text present if you click outside of the issue report.

Some folks may never make mistakes and mis-click, and for that, they'd never know this was a feature.

edit... Also observed that if you edit a file and switch to a different file before a commit, you lose the changes - I would think that's expected - but you're not prompted to commit or advised of the loss of data.

Do you think there should be one?

i don't think there should be one there. Try:

https://fossil.wanderinghorse.net/r/.cwal-dummy/fileedit?filename=s2/mod/regex_js/index.md&checkin=90cf9f978331cdfe

There's a relatively long line around the 3rd paragraph ("the common features are described in the POSIX regex module docs..."). Edit that line and do an SBS diff, and you'll see scrollbars.

What wasn't working before, but was just fixed/uploaded, is the synchronized scrolling of both sides. That required adapting sbsdiff.js a bit so that it could be called multiple times.

While editing a document, the user may get distracted and click outside of the /fileedit, for instance the timeline, and lose their work. Perhaps there could be a notification/popup the work will be lost.

i've made sure to note that in bold in the help tab and the main doc page, but i really want to avoid any sort of popup: once a page shows a certain number of popups via alert() or confirm() (for Firefox: 4 times), the browser starts offering a checkbox on the popup to prevent all further popups from that page, making them useless in the application.

We can implement an HTML-only dialog, but i honestly have no idea how reliable it is to try to stop a user from leaving a page (and we cannot protect against closing of the browser tab). (Detecting and warning about a change is not quite that simple, in any case - see the final paragraph of this post.)

Some folks may never make mistakes and mis-click, and for that, they'd never know this was a feature.

i have little doubt that all of us will mis-click or forgetfully leave the page at some point, but we'll learn quickly not to ;). This app is not really intended to be used for hour-long editing sessions. Quick edits is all it's "really intended" to be used for. Anyone editing a novella needs to be extra careful!

We can't protect users from themselves. Presumably anyone working with this tools is a developer who understands the risks. If they don't, they will soon learn to ;)!

That said... there are vague plans, which i'm not going to detail just yet because they're only half-baked, for using JS's sessionStorage and/or localStorage for storing works-in-progress to try to avoid such problems by restoring the content when the user returns, but there are a few usability hurdles to consider and account for before implementing those. They are considered "version 2" features, but would hypothetically let a user switch back and forth between any number of files/versions, so long as their local edits are "stashed" (probably automatically) in one of those two places before switching files/versions.

See:

• https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage
• https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage

edit... Also observed that if you edit a file and switch to a different file before a commit, you lose the changes - I would think that's expected - but you're not prompted to commit or advised of the loss of data.

That's another case of not even bothering to try to protect a user from themselves. The app actually doesn't know whether you've modified the file or not until it sends the contents to the server and lets the server decide that. The client side does not currently do any tracking of changes, and it literally won't be able to if a user installs a custom editor widget (a feature which already works, as of a few hours ago). The app has no way of knowing how to communicate such details with a custom widget, other than periodically fetching the whole content of the file from that widget and comparing it to an original copy, which would be memory-hungry and computationally expensive (which means "battery hog" on mobile devices).

The only way for the app to be 100% certain that changes have actually be made is to keep 2 copies of the content: store a "pristine" copy when it loads the file and compare that, as needed, against the one in the editor widget. Watching for keypress events and such does not suffice because a user can change the contents and then change it back (Ctrl-Z), in which case there is no real edit. Also, with custom editor widgets, we have no way of knowing how to properly hook in to such events.

Totally understand your justification, and this helps explain it more than I had previously known:

This app is not really intended to be used for hour-long editing sessions. Quick edits is all it's "really intended" to be used for. Anyone editing a novella needs to be extra careful!

Also, I didn't know if you had considered popups/warnings before, so I was presenting this as an improvement only. You're right that we as the uses will eventually learn to carefully click.

Thanks for your thoughts on this matter.

Losing edits will certainly become a point people will make unpleasant noises about before too long, so it needs to eventually be addressed by more than a blurb in the docs. Rather than do a half-baked solution right now i'd rather delay it until i can get a sessionStorage/localStorage solution worked out (it's conceptually simple, but the UI and the figuring out "did something really change?" are the more development-intensive parts). Ideally no sort of dialog/notification will be needed - any local edits will simply be silently stashed when the editor loses focus (noting that a custom editor cannot transparently tell us that), and silently restored if the user visits that file/version combination again. There needs to be UI elements which show the state of the stash, too, to avoid any confusion and allow the user to clean up individual stash entries because of potential storage limits (storage limits are technically unspecified, but my understanding is that at least a few MB is guaranteed on most modern browsers).

See: https://stackoverflow.com/questions/15840976/how-large-is-html5-session-storage

See this post:

https://fossil.wanderinghorse.net/r/.cwal-dummy/info/3fc548a807ece6ac

Simply editing the file and unchecking execute wouldn't permit me to make a commit. Is that the desired outcome?

Simply editing the file and unchecking execute wouldn't permit me to make a commit.

Bug: i was only posting checkbox fields if they were checked (that's a holdover from when the page used a FORM, because checkboxes in forms work that way, but it no longer uses a form). That's fixed locally but i'm in the middle of other edits so will commit it later.

we cannot protect against closing of the browser tab

There is the beforeunload event that JS code can hook to give user a kind of special-case window.confirm not subject to alert/confirm/prompt limits where most of the text is decided by browser:

<html><head><script>
window.addEventListener('beforeunload', (event) => {
  // Cancel the event as stated by the standard.
  event.preventDefault();
  // Chrome requires returnValue to be set.
  event.returnValue = '';
});
</script></head></html>

we cannot protect against closing of the browser tab

There is the beforeunload event that JS code can hook to ...

beforeunload is "advisory," not guaranteed to be honored by a browser. At most it can keep a user from navigating away or reloading, but it can't stop a user from closing a tab: tapping ctrl-w while the notification is being shown (if it's shown at all) will still kill the tab. (Imagine how horrible it would be if marketing people could block us from closing tabs.)

From the bottom paragraph of:

https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers/onbeforeunload

Note also, that various browsers ignore the result of the event and do not ask the user for confirmation at all. In such cases, the document will always be unloaded automatically. Firefox has a switch named dom.disable_beforeunload in about:config to enable this behaviour. As of Chrome 60, the confirmation will be skipped if the user has not performed a gesture in the frame or page since it was loaded. Pressing F5 in the page seems to count as user interaction, whereas mouse-clicking the refresh arrow or pressing F5 with Chrome DevTools focused does not count as user interaction (as of Chrome 81).

i.e., it's not a reliable method. i'm currently working on infrastructure for stashing edits into localStorage when the textarea loses focus, but that will only work for the builtin textarea, not a custom editor widget (which the app doesn't know about, nor know how to interact with).

Achievement Unocked: Edits Un-lost

https://fossil-scm.org/fossil/info/d130f3568793a621

Losing edits will certainly become a point people will make unpleasant noises about before too long, so it needs to eventually be addressed by more than a blurb in the docs.

Management summary: /fileedit now uses window.fileStorage or window.sessionStorage, if available, to stash local edits every time the editor widget fires a "change" event (which essentially means when it loses focus and was modified while it had focus) OR when the "is executable?" toggle is modified. If neither storage mechanism is available then it uses a transient storage object which will not survive a page reload and the editor tab gets a colorful warning about the transient nature of edits. sessionStorage survives "until the tab is closed," persisting across reloads and visits to other pages on the site (i'm not certain if it survives visits to other sites). fileStorage persists for as long as the browser cares to keep it - potentially hours, potentially years.

It is currently hard-coded to keep no more than the most recent 7 (arbitrarily chosen) checkin/file combinations, but that will eventually be improved with a widget which allows managing the list of stored local edits and quickly jumping between those files (internally called the "stash"). If the limit is exceeded, it drops the oldest one (it updates the timestamps when edits are stashed).

When loading a file, the status bar clearly says if it's loaded edits from the stash. Using the editor's "reload" button, or committing changes, will remove that entry from the stash. The UI (and page title) will eventually be improved to clearly indicate when a file has been locally modified but not yet saved.

There is not currently a way in the UI to manually clear the stash, but it can be done via the dev console:

fossil.page.clearStash();

The test server has been updated. Try making edits to multiple files, jumping back and forth, and they should be retained across such jumps, provided the limit of checkin/file combinations is not exceeded.

Sidebar: despite the ability to manage multiple concurrent edits, the infrastructure for this feature only manages checkins of individual files, and expanding it and the UI for multiple files is a task i'm currently up for :/. "Version 2," if at all.

Nice improvement! It looks good so far to me, but it would be nice for others to test it out as well.

One minor thing...not certain if it's related to this latest enhancement or was in previously. The diff doesn't reload until you click the sidebyside or unified diff, even after you've loaded a different file. My expectation would be to clear the diff, so the user would need to select sidebyside or unified links for a fresh reload.

1. Select file to edit
2. Make a minor change
3. Preview the change
4. Diff the change
5. Load a different file
6. Make a minor change
7. Preview the change
8. Click the diff link
9. Notice the diff from the previous file load is still present

It's especially obvious the diff isn't of the file you're viewing and the status message is out of date as well 2020-05-15 19:44:34 UTC: Updated preview.

One minor thing...not certain if it's related to this latest enhancement or was in previously. The diff doesn't reload until you click the sidebyside or unified diff, even after you've loaded a different file.

It's actually always been that way for the diff, but it has gone unnoticed. That applies to the preview as well, but the preview tab "hides" that from you unless auto-refresh is disabled.

i'll change those two tabs to clear their content when the editor's content is replaced via selection of a new file or the "reload/discard" button, but it will probably be tomorrow - i'm shutting down for the evening in a moment.

Thank you again for the continued testing!

@sean: the diff/preview are now cleared when new content is selected.

Management summary: /fileedit now uses window.fileStorage or window.sessionStorage, if available, to stash local edits ...

The UI now includes a widget to easily see and jump between files which have been locally edited:

https://fossil.wanderinghorse.net/screenshots/fileedit-local-edit-list.png

That list is automatically updated whenever the "stashed" edits are modified. The timestamps seen in that list are the last local edit time, not the associated version's checkin time. They are sorted first by name then by checkin version. As before, the list will grow to a max of 7, at which point it will discard the oldest edit, but that number is internally configurable. The local edit stash can be cleared via a button.

If persistent storage is not available, multiple local edits are still supported but will not survive leaving or reloading the page, in which case a "loud" warning is placed under the widget seen in that screenshot.

i don't mind saying, i'm quite pleased with how this little improvement turned out :).

The test server has been updated (be sure to reload).

Sidebar: the branch names seen in that dropdown list are not always available, in which case it instead says something like "?branch?". The app only knows the branch names of the leaves it's loaded in the file selector. It caches them in persistent storage if it can, so that it can keep track of the names for checkins which used to be leaves it knew, but the lifetime of that cache storage is always uncertain (also, the cache does not recognize if a non-leaf was eventually moved to a new branch name, but that's a tiny cosmetic-only problem and can't be solved without adding a new AJAX interface to individually fetch the current branch name for a checkin).

@sean: the diff/preview are now cleared when new content is selected.

Another I didn't really think about - the commit tab. The old manifest stuff is present from the previous commit. The checkboxes are also set to the last used as well.

With your local file enhancement selection, one could edit multiple files almost simultaneously and then commit sequentially, as I demonstrated on your repo. That's a pretty neat feature IMO.

Another I didn't really think about - the commit tab. The old manifest stuff is present from the previous commit. The checkboxes are also set to the last used as well.

The checkboxes, except for the is-executable flag, are retained on purpose. The exec flag is updated to the last known state for a given file when switching files.

The manifest should probably be cleared, but an argument could be made to retain it. Display of the manifest is really just an FYI, though - there's not much a user will be able to to with it. i'll add a flag to only include the manifest in the results if explicitly requested (via checkbox), and that will cut down the commit response size by 95%+.

With your local file enhancement selection, one could edit multiple files almost simultaneously and then commit sequentially,

Sequentially, yes, but not all together. That would not only require a major overhaul of the new infrastructure, but also the UI, and would raise troubling questions about how to handle edits made from different checkins. It will certainly be a requested feature the first time someone queues up 3+ edits locally, but that feature is not on the TODO list because of the amount of effort involved.

The checkboxes, except for the is-executable flag, are retained on purpose.

Fair enough. Your reasoning is good enough for me.

Sequentially, yes, but not all together.

Yep, I knew that undertaking would be significant. Editing locally exists for the need to commit once for multiple files.

The manifest should probably be cleared, but an argument could be made to retain it.

Just uploaded...

• The manifest is now cleared when the editor gets new content.
• Whether or not the manifest is returned with the commit response is now controlled by a checkbox, defaulting to off. In general the manifest is not useful here except for testing, and it can be quite large (78k for the fossil core trunk).
• The preview/diff are now cleared after a non-dry-run commit.
• A checkin's branch name is now delivered to the client when loading file content so that we always have the branch name for locally-stashed edits, even if their checkin later becomes a non-leaf. Previously the UI only had branch names for leaf checkins.
• Other minor verbiage improvements and internal/invisible cleanups.

THANK YOU once again for all of your testing. i'm exceedingly happy to report that of the many bugs and improvements you guys have discovered or prompted, none of them were in the manifest generation/handling parts. Rather, all of them were either UI-specific or had to do with how ajax request parameters were being handled. i.e. none of them revealed a problem which posed any risk to a repository's integrity.

Stephan,

Thanks for this feature. I have tested it and I like it pretty much and is really useful for our Fossil common use in documentation. We usually do quick edits on CodiMD and then we improve them on a Fossil repository using offline Markdown tools and transform the files to several formats, including Markdeep. Your /fileedit page with syntax highlighting, preview, web buttons and all the goodies your are putting on it, brings the gap between online and offline editing and I hope it becomes part of the official Fossil features.

With the embedded docs, the Web Document Format, the /fileedit features, I think that Fossil can fill a niche for writing and even reproducible research, that is not well addressed by more overcomplex and popular tools. The long discussion in this thread shows the great interest in this feature.

Cheers and keep the good work,

Offray

Thank you for continuing to work from all of us, and quickly working on the feedback you receive.

Is it possible to add a list of files in the local storage, then load that file? Currently I need to know the file name to load it from the local storage. I guess this would be akin to fossil diff on the cli.

Is it possible to add a list of files in the local storage, then load that file

The answer depends on what you mean by "local storage":

• If you mean the local filesytem: JavaScript is, thank goodness, generally not permitted to access the local filesytem. Chrome has an API for doing so, but That Way Lies Madness. HTML has a file upload element which can send data to a server but cannot, AFAIK, provide access to the data to JS. HTML5 offers drag/drop support which can be used to drop content into the browser, and JS can access that data, but it's not a feature i would currently be comfortable adding for this app. To get a local file's contents into the editor, i suggest copy/paste from a text editor. "One of these days" i could imagine tinkering with drag/drop support, but it's not a feature i see myself using, so don't have any motivation to go figure it out :/.
• If you mean the JS localStorage object: that list is in the new drop-down selection widget on the file contents tab.

Hi,

Yes, I meant the latter and sorry for not being more clear. I see this working as well and meets my expectations. I probably opened a file and didn't make any change, therefore it wouldn't have appeared in the localstorage object list.

Sorry for the noise.

Thanks again for your implementation on this. I hope others will soon offer their feedback soon.

Couple questions...

1. Will this be off by default, and the admin user will need to set it up with fileedit-glob?
2. If this is primarily intended with txt/md editing, why the execute flag? I don't have any concerns with it and like that you added it, just curious.

Will this be off by default, and the admin user will need to set it up with fileedit-glob?

Correct. If you visit the page while logged in, but no glob is set, it shows you a loud warning about how to activate it by setting the glob. Not everyone is comfortable with the idea of opening their repo to modification via browsers, especially in projects where the users cannot be trusted to avoid the temptation to "just make a quick/small fix in this source code..." via the browser (where they cannot possibly test such changes before committing them). Hopefully the glob whitelist approach will be flexible enough to suit most peoples' needs.

If this is primarily intended with txt/md editing, why the execute flag? I don't have any concerns with it and like that you added it, just curious.

The web interface for this feature is intended to be used for exactly those types of files, but it's not my intent to force people to follow that policy. As movie character Blade so wisely said:

There's always [someone] trying to ice-skate uphill.

If that feature were missing, at least one person who insists on ice-skating uphill (by editing shell scripts via the web) would ask for it to be added.

Also, the feature was initially developed and tested on the CLI, where the exec bit makes more sense, so there was no compelling reason to leave out that support there. However, it does not support symlinks (which fossil records as file permission "l") and trying to edit a symlink will result in an error.

Just uploaded to the test server:

• All /fileedit/ajax requests now do CSRF (Cross-Site Request Forgery) checks and fail if a CSRF violation seems likely. This affects all features except the initial loading of the page.
• Other minor internal tweaks, but CSRF is the biggie.

Please let me know if it starts failing for you. It should not fail unless your browser is configured to completely disable sending of HTTP Refer[r]er data, as CSRF checking requires that feature.

With network.http.sendRefererHeader checks it seems that fileedit works only if network.http.sendRefererHeader=2. Values 0 and 1 make fileedit emit warnings.

Should not it work with value 1 as well? Thanks.

i had never heard of that setting until yesterday, so i'm not well-versed in it, but according to the page you(?) linked to in that edit:

0 = never send the header

That's expected. CSRF protections require referrer info. Richard mentioned in /forumpost/f8ca6368e0 (and its follow-up responses) that several fossil features don't work if sending of refer[r]er is disabled.

1 = send the header only when clicking on links and similar elements

That's literally not possible with XHR requests, as those are not directly associated with a user action (though most, but not all, happen in indirectly in response to a click).

That's potentially an issue. Very nearly all of fossil's current features act upon a directly click from a user, either a hyperlink or a button, with very little Ajax/XHR activity and no write-mode XHR activity.

Based on what i've seen this morning (while implementing that change in response to Richard mentioning it in that other thread), fossil only normally protects write-mode operations with CSRF, and fossil has no write-mode XHR operations, thus fileedit's use of CSRF protections, in conjunction with XHR, is the first time this has come up in fossil.

i don't have a good answer. i'm not a fan of using refer[r]er at all, simply because it can be, and often is, disabled, but the CSRF protections require it and Richard will certainly require that fileedit be protected against CSRF.

It seems that we'll simply have to document that fileedit requires, in order to implement CSRF protections, that the sending of the referrer be enabled.

Edit: Refer[r]er (aarrrgg, that misspelling hurts my brain!) cannot be explicitly set as a request header on XHR connections - it's classified as a "protected" header to prevent spoofing from XHR.

Sidebar: The JSON API also has write operations, but its implementation predates CSRF checks by several years and it has never been updated to account for CSRF. i incorrectly thought, until a few hours ago, that CSRF protections happened at level below where that API was concerned with. That needs to be revisited, at the risk of breaking any JSON-using clients (the core tree does not use that API). Edit: OTOH, the JSON API already requires authentication with each request which performs write ops, and is intended to be usable with sessionless clients such as wget and curl. i'll need to take a closer look at the implications of adding CSRF checks to it.

Just uploaded to the test server:

... also just uploaded...

• When XHR/Ajax requests are in transit, all input elements (buttons, text fields, checkboxes, etc.) are disabled and the whole page gets a "waiting" cursor.

The lack of visual feedback during network waits (my hoster isn't always fast) was irritating me. Adding the disable/wait to only the elements relevant to a given request would be a huge pain to implement and maintain, so it's a global all-or-nothing notification lasting as long as any XHR requests are in transit.

This is easy to see in action by using FF/Chrome's dev tools to throttle the network operations to "GPRS" speed.

Perhaps I am misunderstanding how network.http.sendRefererHeader=1 works.

Perhaps I am misunderstanding how network.http.sendRefererHeader=1 works.

My understanding is that level 1 is essentially an interactivity check: it will send the referrer when it knows, via mouse/keyboard interaction, that the request is part of a human interaction. XHR requests don't qualify for that because there's no real way to know if they're triggered as part of a human interaction. That level protects a user by keeping sneaky background network requests from passing on the referrer, thereby potentially bypassing CSRF and similar checks. With level 1, XHR requests still work but don't include that header, and JS code is prohibited (regardless of the level) from setting that header on XHR connections.

Apparently there's a standard way for the page to override that preference, such that it can specify a policy on whether/how to pass on referrer info:

https://www.w3.org/TR/referrer-policy/

specifically:

https://www.w3.org/TR/referrer-policy/#referrer-policy-same-origin

Whether or not that really applies to XHR requests, in addition to interaction, is unclear but looks like it will be easy to test by passing it on as a response header from /filepage:

https://www.w3.org/TR/referrer-policy/#referrer-policy-delivery

i will try that out later and report back. If that works then we have no issue at all with referrer headers vs. XHR.

My understanding is that level 1 is essentially an interactivity check: it will send the referrer when it knows, via mouse/keyboard interaction, that the request is part of a human interaction.

That appears to be the case. In level 1, FF does not send a referrer when requesting CSS or JS files, even if the Referrer-Policy response header from the containing page says it's okay.

Whether or not that really applies to XHR requests, in addition to interaction, is unclear...

Testing shows that Firefox (v74) is ignoring the Referrer-Policy HTTP header entirely, no matter what its value, if network.http.sendRefererHeader is set to 1 or less.

If it's set to 2 then Firefox does honor the Referrer-Policy, as can be seen by changing the policy from same-origin to origin: the latter sends a referrer equal to only the scheme and domain name, minus the SCRIPT_NAME part and remaining request path. That breaks fossil's CSRF check (which compares g.zBaseURL to the referrer), and also breaks the use of HTTP_REFERER to fish out the page name for the style.css case (discussed in the style.css thread).

However, with level 2 then FF automatically sends a referrer, so the Referrer-Policy response header doesn't give us anything except the ability to control, to a limited degree, how much info FF sends us as the Referer with each request.

Summary: fossil apparently cannot, at least in FF, control whether XHR requests will send a Referer request header. In order to use CSRF protections in conjunction with XHR, clients will have to have this set to level 2 (which is the default).

That's unfortunate, but it seems better than foregoing CSRF altogether. We can judge the real impact better if/when people start saying that fileedit is report "CSRF violation" errors in their real-world setups.

A very, very pedantic comment regarding the help page...

I think "checkin" has been eradicated and replaced with "check-in" on virtually all help messages and in the webUI. My suggestion would be to update the few lines to "check-in" on your help & tips section.

My suggestion would be to update the few lines to "check-in" on your help & tips section.

That is indeed pedantic, but not wrong. The help tab has been updated:

https://fossil-scm.org/home/info/91948d3afa52c9c5

Noting that (1) the test server probably won't be updated for that change and (2) the "checkin" URL parameter name is still "checkin" (as it is on several other pages).

... we now have a feature in a branch which allows editing of individual files via the web interface...

Update: /fileedit is now in trunk, so the /fileedit test server will be allowed to slowly die off, or perhaps be used for testing any further major changes (none are currently planned).

Visiting /fileedit in one's own repositories will initially not work: the page must be explicitly activated by a setup user. Until that happens, the page shows the user a message explaining how to activate it.

My many thanks to those of you who hit it with rocks to try to break it. The only "potentially tragic" bug which was uncovered was in the EOL conversions, where extra carriage-returns could have been injected when explicitly requesting Windows-style EOLs. Aside from that, all bugs were in the "higher-level" code, posing no risk to user content. Your feedback led to many improvements in overall usability as well.

That said: this is not the end! Feel free to continue contributing feedback, either in this thread or new posts.

Is it the right choice to export the "fileedit-glob" configuration setting so that it flows through to child repositories on a clone, or is synced using "fossil configure sync /project"? (See configure.c line 153.) Or would it be better to let local admins set up fileedit-glob for themselves?

Is it the right choice to export the "fileedit-glob" configuration...

It should very probably be set individually per clone, rather than propagate. i misunderstood that each config setting should be in some group or other. i'll remove it from that list later today unless there's a reason you prefer to make it syncable.

It was very specifically not made versionable, to avoid that a non-admin change the policy.

Regarding your permissions check change here:

https://fossil-scm.org/home/info/f2312397802722a2

That access check isn't quite right: the ajax bits do that check themselves and respond with a JSON-format error if anything is amiss (which includes CSRF validation). The client side expects only JSON from them, and won't be able to deal with an HTML response. (That said, that case "should" only trigger if a user's perms are changed while they're using that page.) i'll reformulate that to move the ajax dispatch check above the main page's write-perms check, but leave that check in place with your reformulation of it.

I intentionally moved the check early and to a single place to facility security auditing. As you say, the error shouldn't come up in practice. AJAX should only see an HTML response if there is mischief afoot.

But if you want, why not leave the early check in place but change it to return a AJAX-appropriate error if P("name") is not NULL.

It is ok to do the security checks twice - once at the top of /fileedit, and again within each JSON handler subroutine. In fact, I would prefer it that way, for defense-in-depth.

But if you want, why not leave the early check in place but change it to return a AJAX-appropriate error if P("name") is not NULL.

That sounds fair. i'll make that change in a moment.

Hi Stephan,

Using Fossil preview 2.12 on my repo with the Ardoise skin on draft2, going to /fileedit, the theme changes to the two-tone gray layout. This is a bit surprising, but it functioned just fine and allowed me to do the editing.

Did you intend for the skin layout to change with this skin only on the /fileedit page?

Did you intend for the skin layout to change with this skin only on the /fileedit page?

That version doesn't define any new colors at all except for in the error/warning message CSS classes, and i strongly suspect that you're seeing a caching issue: i saw something similar several times while swapping out skins during testing on the dummy repo. i just tested this with the latest tip locally and see the expected colors with that skin.

Hi Stephan,

Thanks for your eyes on this. You're right, this very well could be a cache issue, but I don't ever recall using draft 2 and this skin. I'll create a new Firefox profile and re-test again soon.