Page 1 of 1

"Cancel" saves ABE ruleset changes even with syntax errors

Posted: Fri Aug 19, 2011 1:45 am
by al_9x
shouldn't save at all

Fx 6.0, NS 2.1.2.7rc2

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Fri Aug 19, 2011 10:47 am
by Giorgio Maone
ABE rules are saved live as soon as you exit the editing field.
Cancel does nothing, they're already persisted.

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Fri Aug 19, 2011 11:40 am
by al_9x
Giorgio Maone wrote:ABE rules are saved live as soon as you exit the editing field.
Cancel does nothing, they're already persisted.
1) If you don't save a syntax error on ok close, why should you save it on cancel close? At the very least it should get the same treatment.

2) A user does not know the rules are persisted on loss of focus, he just clicks cancel, and no one would expect for cancel to save, it means the opposite.

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Sat Aug 20, 2011 1:06 am
by al_9x
You already addressed the issue of closing with error. I think cancel should really cancel (i.e. undo all changes) but if it's going to save it should alert about errors, same as ok.

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Sun Aug 21, 2011 1:54 pm
by therube
1) I really would not have attributed the Cancel button to be part of the ABE dialog (as in I would have removed erroneous rules by blanking out <if made aware of>).
2) You may, momentarily see the rules go "red" (though that is irrelevant).
3) Since there is a Cancel option clickable, I would have expected it to not record ABE changes. (Suppose the same could be said with the close-X button, perhaps?)

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Sun Aug 21, 2011 2:06 pm
by Giorgio Maone
I agree that hitting the Cancel button should at least be protective against accidentally save a syntax error as much as "OK", so I'm putting that one in the bugs bag.
On the other hand, turning the whole ABE ruleset management in a "double-buffered" one, so that all the changes can be undone on Cancel, is too much of a change to be justified in light of the rewrite planned for 3.0 (where most if not all the preferences will likely be "live").

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Tue Aug 23, 2011 5:30 pm
by Giorgio Maone

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Tue Aug 23, 2011 7:37 pm
by al_9x
Giorgio Maone wrote:Please check latest development build 2.1.2.7rc3, thanks.
  1. the modal dialog is only needed when closing, as that's the only way to alert at that point. When saving without closing, it's annoying and unnecessary, the non modal error display (of prior versions) is preferable.
  2. clicking don't save, now removes your changes - that's not at all intuitive, "don't save" implies the changes are not persisted to the backing store, not that the editor will lose them. Who would ever click save on a syntax error? Which is what one has to do now to fix it.
  3. (preexisting issue) a save loses the scroll position - would be helpful if the scroll position and even the insertion point were retained.

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Tue Aug 23, 2011 7:44 pm
by Giorgio Maone
al_9x wrote:
Giorgio Maone wrote:Please check latest development build 2.1.2.7rc3, thanks.
  1. the modal dialog is only needed when closing, as that's the only way to alert at that point. When saving without closing, it's annoying and unnecessary, the non modal error display (of prior versions) is preferable.
  2. clicking don't save, now removes your changes - that's not at all intuitive, "don't save" implies the changes are not persisted to the backing store, not that the editor will lose them. Who would ever click save on a syntax error? Which is what one has to do now to fix it.
  3. (preexisting issue) a save loses the scroll position - would be helpful if the scroll position and even the insertion point were retained.
I dissent with both 1 and 2: this new alerting style makes self-evident that changes are saved (or reverted) as soon as you leave the editor box (as it already happened). Since this underlying behavior is not gonna change in 2.x, current alerts account more faithfully for what is really happening under the hood.

Regarding 3, it would be too much of a hassle for an EOL branch.

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Tue Aug 23, 2011 8:20 pm
by al_9x
The current "don't save" behavior promotes data loss. It is completely unintuitive to click save on a syntax error. How do you expect anyone to know to do that? People will click don't save, expecting to be able to fix the error and instead lose their edits.

About 3.x,

Do you plan on releasing 3.x before electrolysis? If not, then it may be to early to give up on 2.x. The rapid release process does not mean the rate of Fx development got faster, only the release of completed features. A major thing like electrolysis is likely to be delayed significantly, keeping NS in limbo.

Re: "Cancel" saves ABE ruleset changes even with syntax erro

Posted: Tue Aug 23, 2011 8:34 pm
by Giorgio Maone
al_9x wrote:The current "don't save" behavior promotes data loss. It is completely unintuitive to click save on a syntax error.
OK, we'll go back to lie and prompt on exit only.
al_9x wrote:Do you plan on releasing 3.x before electrolysis?
Absolutely. The sooner, the better. Notice that electrolysis is already there (since Firefox 4), but web content has not been switched to a different process yet on the desktop because it would break most if not all the Firefox desktop UI, which relies on chrome and content to live in the same process. However, applications designed to be electrolysis-compatible (like NSA) can work just fine on current Firefox desktop version.

The reason why NoScript 3 has not been released yet for the desktop is that desktop UI, ABE and Script Surrogates are not ready yet. But we definitely don't need (and don't want) to wait for a multi-process desktop Firefox release: actually we strongly hope to release way before.