The default bug view has changed. See this FAQ.

Highlighter should be useable via keyboard.

VERIFIED FIXED in Firefox 10

Status

()

Firefox
Developer Tools
P1
normal
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: davidb, Assigned: past)

Tracking

({access})

unspecified
Firefox 10
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [highlighter][minotaur][best: 3h: likely: 6h; worst 2d][fixed-in-fx-team])

Attachments

(1 attachment, 3 obsolete attachments)

Comment hidden (empty)
(Reporter)

Comment 1

6 years ago
STR: unplug your mouse.
Blocks: 663830
Whiteboard: [highlighter][minotaur]
Currently it doesn't seem to be possible to focus the HTML panel via the keyboard. We should also have global key commands for selecting previous/next sibling and parent/first-child.
Depends on: 669659
left: parent
right: first child
up: prev sibling
down: next sibling
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
Whiteboard: [highlighter][minotaur] → [highlighter][minotaur][best: 3h: likely: 6h; worst 2d]
robcee, if you have correctly keyed dependency on bug 669659, shouldn't it really be bug 644733?
(In reply to Wayne Mery (:wsmwk) from comment #4)
> robcee, if you have correctly keyed dependency on bug 669659, shouldn't it
> really be bug 644733?

Actually I believe this was an off-by-one error. The bug this depends on is about improving the inspector keybindings.
Depends on: 669658
No longer depends on: 669659
whups, yes indeed. Thanks Wayne and Panos!
Priority: -- → P1
I just started working on this.
Assignee: rcampbell → past
Created attachment 557221 [details] [diff] [review]
First draft

This patch seems to work fine and not break anything. I'll be adding tests tomorrow, but just wanted to make sure I'm on the right track.
Attachment #557221 - Flags: feedback?(rcampbell)
Created attachment 557494 [details] [diff] [review]
Patch with tests

Got the tests ready, so let's turn this into a proper review.
Attachment #557221 - Attachment is obsolete: true
Attachment #557494 - Flags: review?(rcampbell)
Attachment #557221 - Flags: feedback?(rcampbell)
Comment on attachment 557494 [details] [diff] [review]
Patch with tests

In:

inspector.js

line 75:

+            let selection = this.selection;

Don't think we need the variable declaration.

line 79:

+              node = gBrowser.selectedBrowser.contentWindow.document.documentElement;

node = this.defaultSelection;

line 88:

+            selection = this.selection;

still don't need it! Though if it's convenient and saves space, you could cache it outside the switch condition.

nice test! r+ with the above fixed. I *think* that test looks good. I'll give it a look-see tomorrow.
Attachment #557494 - Flags: review?(rcampbell) → review+
Created attachment 557780 [details] [diff] [review]
Patch v3

(In reply to Rob Campbell [:rc] (robcee) from comment #10)
> Comment on attachment 557494 [details] [diff] [review]
> Patch with tests
> 
> In:
> 
> inspector.js
> 
> line 75:
> 
> +            let selection = this.selection;
> 
> Don't think we need the variable declaration.
> 
> line 79:
> 
> +              node =
> gBrowser.selectedBrowser.contentWindow.document.documentElement;
> 
> node = this.defaultSelection;
> 
> line 88:
> 
> +            selection = this.selection;
> 
> still don't need it! Though if it's convenient and saves space, you could
> cache it outside the switch condition.
> 
> nice test! r+ with the above fixed. I *think* that test looks good. I'll
> give it a look-see tomorrow.

All fixed, and the defaultSelection even simplified the test a bit.
Attachment #557494 - Attachment is obsolete: true
Attachment #557780 - Flags: review?(rcampbell)
Comment on attachment 557780 [details] [diff] [review]
Patch v3

I'm ok with these changes. We should get gavin to give it a once-over.
Attachment #557780 - Flags: review?(rcampbell)
Attachment #557780 - Flags: review?(gavin.sharp)
Attachment #557780 - Flags: review+
Created attachment 564142 [details] [diff] [review]
[in-fx-team] Patch v4

Rebased and updated, after the the inspector move into the devtools directory. Gavin, I can take this off your plate now, but if you were already in the middle of the review or still want to do it regardless, you are most welcome. 

Try run results at:

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=ba831a3f48ca
Attachment #557780 - Attachment is obsolete: true
Attachment #557780 - Flags: review?(gavin.sharp)
Whiteboard: [highlighter][minotaur][best: 3h: likely: 6h; worst 2d] → [highlighter][minotaur][best: 3h: likely: 6h; worst 2d][land-in-fx-team]
Comment on attachment 564142 [details] [diff] [review]
[in-fx-team] Patch v4

https://hg.mozilla.org/integration/fx-team/rev/f425a8109714
Attachment #564142 - Attachment description: Patch v4 → [in-fx-team] Patch v4
https://hg.mozilla.org/mozilla-central/rev/f425a8109714
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10

Comment 16

6 years ago
Verified Fixed using Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111006 Firefox/10.0a1.
Status: RESOLVED → VERIFIED
Whiteboard: [highlighter][minotaur][best: 3h: likely: 6h; worst 2d][land-in-fx-team] → [highlighter][minotaur][best: 3h: likely: 6h; worst 2d][fixed-in-fx-team]
You need to log in before you can comment on or make changes to this bug.