Closed
Bug 949365
Opened 11 years ago
Closed 11 years ago
[Australis] Ctrl-K/Cmd-K doesn't focus the search bar
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
VERIFIED
FIXED
Firefox 29
People
(Reporter: smontagu, Assigned: jaws)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
2.40 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
In a fresh nightly build the behaviour of Cmd-K/Crtl-K has changed. Instead of focusing the search bar it opens google.com (or whatever is selected as default search engine). If I remove the search bar via Toolbars | Customize, Cmd-K does nothing. Seen on both Linux and OSX, with both a previously existing profile and a newly created one.
Reporter | ||
Comment 1•11 years ago
|
||
N.B. This is not a dupe of bug 901207, since I have the fix for that in my tree -- could it be regression from that fix?
Flags: needinfo?(jaws)
Comment 2•11 years ago
|
||
Looks like it is: > - if (searchBar && window.fullScreen) ... > + if (searchBar && window.fullScreen) { > FullScreen.mouseoverToggle(true); > - if (searchBar) > searchBar.select();
Blocks: 901207
Assignee | ||
Comment 3•11 years ago
|
||
Hm, I'm not sure how that got checked in. I was pretty sure that the code I had for this was what I have attached as a patch here. Thank you for filing the bug and getting attention to this so quickly!
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attachment #8346569 -
Flags: review?(mconley)
Flags: needinfo?(jaws)
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 4•11 years ago
|
||
Added a test because I hate regressions.
Attachment #8346569 -
Attachment is obsolete: true
Attachment #8346569 -
Flags: review?(mconley)
Attachment #8346573 -
Flags: review?(mconley)
Comment 5•11 years ago
|
||
Comment on attachment 8346573 [details] [diff] [review] Patch with test Review of attachment 8346573 [details] [diff] [review]: ----------------------------------------------------------------- r=me. For brownie points, if no test exists for the shortcut opening the webpage if the control has been entirely removed, can you file a followup bug for that? Non-trivial because it needs a dummy search provider because otherwise we'd access the network, which is a no-no in tests. Also, I missed some nits originally... ::: browser/base/content/browser.js @@ +2951,5 @@ > // The searchBar gets moved when the overflow panel opens. > searchBar = this.searchBar; > searchBar.select(); > openSearchPageIfFieldIsNotActive(searchBar); > }); This callback is identical to the one above it. As it's an arrow function anyway, assigning it to a local variable beforehand would simplify this code. @@ +2952,5 @@ > searchBar = this.searchBar; > searchBar.select(); > openSearchPageIfFieldIsNotActive(searchBar); > }); > return; After the return in the previous conditional, nix the 'else' part of the else if. No else after return ever, please.
Attachment #8346573 -
Flags: review?(mconley) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Filed bug 949626 for the follow-up. https://hg.mozilla.org/integration/fx-team/rev/4bc1aa287621
Depends on: 949626
Assignee | ||
Updated•11 years ago
|
Summary: Ctrl-K/Cmd-K doesn't focus the search bar → [Australis] Ctrl-K/Cmd-K doesn't focus the search bar
https://hg.mozilla.org/mozilla-central/rev/4bc1aa287621
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Updated•10 years ago
|
QA Contact: cornel.ionce
Comment 9•10 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0 Verified as fixed on Firefox 29 beta 1 (build ID:20140318013849).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•