Closed
Bug 734365
Opened 12 years ago
Closed 12 years ago
Rule view focus management needs an overhaul
Categories
(DevTools :: Inspector, defect, P2)
DevTools
Inspector
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 15
People
(Reporter: miker, Assigned: dcamp)
References
Details
(Whiteboard: [ruleview][fixed-in-fx-team])
Attachments
(1 file, 1 obsolete file)
25.77 KB,
patch
|
paul
:
review+
|
Details | Diff | Splinter Review |
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?
Comment 1•12 years ago
|
||
Would it be possible to listen to "click", and stop the event if the selection start (onmousemove + selection non-empty)?
Assignee | ||
Comment 2•12 years ago
|
||
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).
Assignee: nobody → dcamp
Status: NEW → ASSIGNED
Attachment #626640 -
Flags: review?(paul)
Attachment #626640 -
Flags: feedback?(mratcliffe)
Reporter | ||
Comment 3•12 years ago
|
||
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+
Comment 4•12 years ago
|
||
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 5•12 years ago
|
||
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+
Assignee | ||
Comment 6•12 years ago
|
||
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 #626640 -
Attachment is obsolete: true
Attachment #628491 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #628491 -
Flags: review? → review?(paul)
Comment 7•12 years ago
|
||
Comment on attachment 628491 [details] [diff] [review] v2 It works. And it feels solid.
Attachment #628491 -
Flags: review?(paul) → review+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/85c153eea9d4
Whiteboard: [ruleview] → [ruleview][fixed-in-fx-team]
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/85c153eea9d4
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•