Closed Bug 734365 Opened 9 years ago Closed 9 years ago
Rule view focus management needs an overhaul
The rule view currently triggers it's editor whenever e.g. a name or value node receives focus. This method has it's shortcomings as evidenced by mousing down to start selecting text. We should change this so that the property editor is not triggered when mousing down to select text. Not sure how this is best done ... maybe using mouseup instead of focus to trigger editing?
Would it be possible to listen to "click", and stop the event if the selection start (onmousemove + selection non-empty)?
The attached patch makes a few changes: a) Editors are only started on click, not focus events. We still use focus management to move between fields, but initial triggering of the editor is only done with a click. This keeps keynav working well. b) The editor explicitly takes care of its commit handler etc. while handling events. Before this patch it would cause blurs and rely on blur handlers to apply things asynchronously. c) As a result of these changes, we were able to get rid of a few hacks (_selectionMode and the focus backstop crud). d) Selection now works from anywhere (before this patch, you can't start a selection inside a property name).
Comment on attachment 626640 [details] [diff] [review] v1 Awesome, especially because it gets rid of a gazillion ugly hacks at the same time as making the editor more accessible and maintainable. 100% f+ from me.
Attachment #626640 - Flags: feedback?(mratcliffe) → feedback+
I see a cursor now: http://i.imgur.com/RnZEk.png STR: click on a property name to bring the editor. Press ESC, then down.
Comment on attachment 626640 [details] [diff] [review] v1 Review of attachment 626640 [details] [diff] [review]: ----------------------------------------------------------------- About the "ugly workaround". I am not sure this is really useful, as the dotted line is visible just for a very short instant (but I'm ok with this). r+ if you can fix the cursor.
Attachment #626640 - Flags: review?(paul) → review+
This patch changed just enough that I'm going to reask for review: * Fixed the caret problem by focusing the span after the editor is cancelled. * Refactored some tests a little bit.
Attachment #628491 - Flags: review? → review?(paul)
Comment on attachment 628491 [details] [diff] [review] v2 It works. And it feels solid.
Attachment #628491 - Flags: review?(paul) → review+
Whiteboard: [ruleview] → [ruleview][fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
You need to log in before you can comment on or make changes to this bug.