Closed Bug 967044 Opened 6 years ago Closed 5 years ago

Make Home and End keys scroll console output area and autocomplete popup


(DevTools :: Console, defect, P3)



(Not tracked)

Firefox 34


(Reporter: rcampbell, Assigned: veeti.paananen, Mentored)


(Whiteboard: [lang=js][mentor=robcee])


(1 file, 2 obsolete files)

1. when a user has focused the input line but has not entered anything, Home and End keys should scroll the output area to the top and bottom respectively.

2. if a user has typed something and an autocomplete popup is onscreen, the Home and End keys should scroll the autocomplete popup to the top and bottom (respectively).

3. If a user has typed something in the input area and there is no autocomplete popup, the input cursor should move to the start of the line with home key and end of line with end key.
Assignee: nobody → rcampbell
No longer blocks: devtools-shortcuts
no time, happy to help someone else fix this.
Assignee: rcampbell → nobody
Whiteboard: [mentor=robcee][lang=js]
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
Mentor: rcampbell
Whiteboard: [mentor=robcee][lang=js] → [lang=js]
Attached patch 967044.patch (obsolete) — Splinter Review
Attachment #8446327 - Flags: feedback?(rcampbell)
Comment on attachment 8446327 [details] [diff] [review]

Review of attachment 8446327 [details] [diff] [review]:

I think this looks right. Nice patch.

Can we have a test too? :)
Attachment #8446327 - Flags: feedback?(rcampbell) → feedback+
Whiteboard: [lang=js] → [lang=js][mentor=robcee]
Thanks. What sort of tests? Mochitest? Any pointers?
**** Bugzilla. Am I just doing something wrong or is the "Need more information from..." checkbox not working?
Flags: needinfo?(rcampbell)
it's working. Sorry for the delay.

Yes, browser mochitests.

Look in the browser/devtools/webconsole/test directory for examples.

docs are here:
Flags: needinfo?(rcampbell)
I tried multiple times to set needinfo using the checkbox below the comment field to no effect. It only appeared once I used "set flags" from the top box thing.

Anyway, I'm attaching another patch with some tests I managed to cobble together. It tests the autocompletion and input field fine, but I'm not sure how to test scrolling the output panel. It also complains about some leaks for reasons beyond me. Some advice would be great.
Attached patch 967044-2.patch (obsolete) — Splinter Review
Attachment #8446327 - Attachment is obsolete: true
Attachment #8456161 - Flags: feedback?(rcampbell)
Comment on attachment 8456161 [details] [diff] [review]

Review of attachment 8456161 [details] [diff] [review]:

::: browser/devtools/webconsole/test/browser_webconsole_home_end.js
@@ +15,5 @@
> +  }, true);
> +}
> +
> +function consoleOpened(aHud) {
> +  HUD = aHud;

there's your leak. If you need to capture this, declare it at the top of the file with a |let hud | as you did with popup.


for an example of good setup at the beginning of a test.

(you could also just add your home and end key tests to that file since it's testing this kind of behavior)

@@ +17,5 @@
> +
> +function consoleOpened(aHud) {
> +  HUD = aHud;
> +  info("web console opened");
> +  jsterm = HUD.jsterm;

same here. Don't declare globals.

@@ +19,5 @@
> +  HUD = aHud;
> +  info("web console opened");
> +  jsterm = HUD.jsterm;
> +  popup = jsterm.autocompletePopup;
> +  inputNode = jsterm.inputNode;

and here.
Attachment #8456161 - Flags: feedback?(rcampbell)
OK, thanks for the help. I added new assertions to two existing test cases. I also realized that the input field already handles home/end by itself, so I removed the special handling for that.

Attaching a new patch.
Attachment #8456161 - Attachment is obsolete: true
Attachment #8457411 - Flags: review?(rcampbell)
Comment on attachment 8457411 [details] [diff] [review]

Review of attachment 8457411 [details] [diff] [review]:

this looks good! Thanks!

R+ with a successful run through try.
Attachment #8457411 - Flags: review?(rcampbell) → review+
Veeti, do you have Try server access?
Assignee: nobody → veeti.paananen
Flags: needinfo?(veeti.paananen)
Flags: needinfo?(veeti.paananen)
lgtm! ready to land.
Keywords: checkin-needed
Keywords: checkin-needed
Whiteboard: [lang=js][mentor=robcee] → [lang=js][mentor=robcee][fixed-in-fx-team]
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [lang=js][mentor=robcee][fixed-in-fx-team] → [lang=js][mentor=robcee]
Target Milestone: --- → Firefox 34
Verified fixed:
repro: 2014-07-27-03-02-04-mozilla-central-firefox-34.0a1.en-US.linux-x86_64
QA Whiteboard: [bugday-20140706]
QA Whiteboard: [bugday-20140706] → [bugday-20140806]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.