Closed Bug 1054931 Opened 5 years ago Closed 5 years ago
Ctrl-K focuses an invisible search bar in about:newtab in "blank" mode
STR: - Ctrl-T (opens up a new tab) - Switch (using the top-right gears button) to "blank" mode - Hit Ctrl-K (invisible search box gets focused) - Hit down arrow Result: - The search suggestions are shown in a dropdown menu, even though the initial search field is hidden.
Element with id "newtab-search-container" gets a page-disabled="true" attribute which sets its opacity to 0, but the search input is still in the page and gets focused when CMD + K is pressed. This is where the "page-disable" attribute is added / removed: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/page.js#146 . We will also need a new test in browser/base/content/test/newtab/browser_newtab_search.js for this particular case.
Assignee: nobody → abardas
Status: NEW → ASSIGNED
Thanks for taking this, Alex. I mentioned something about e10s when we spoke briefly about this, but actually I think we don't have to worry about that right now. That's because this module called NewTabUtils.jsm tracks a bunch of state related to all the newtab pages. We should just use that in this bug. Specifically you want to use the AllPages.enabled getter instead of checking the "page-disable" attribute on the customize button in the DOM: http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/NewTabUtils.jsm?rev=de9d70a14a16#240 So I think this bug should be pretty simple (aside from the test). browser.js doesn't import NewTabUtils.jsm yet, so you'll need to add a lazy import for it.
Thanks for the info Drew. Hopefully, unit tests cover all cases now.
Attachment #8475249 - Flags: review?(adw)
Comment on attachment 8475249 [details] [diff] [review] Ctrl / Cmd + K will open an about:home page with the search bar focused when newtab is in blank mode Review of attachment 8475249 [details] [diff] [review]: ----------------------------------------------------------------- Cool, thanks. Please push to try and then add checkin-needed if try is OK.
Attachment #8475249 - Flags: review?(adw) → review+
Thanks Drew. I'm waiting for the test results here: https://tbpl.mozilla.org/?tree=Try&rev=0a44b508fcbb. I'll update the bug shortly after they finish running.
(In reply to Alex Bardas :alexbardas from comment #5) > Thanks Drew. I'm waiting for the test results here: > https://tbpl.mozilla.org/?tree=Try&rev=0a44b508fcbb. I'll update the bug > shortly after they finish running. I've canceled those because they were not running on enough problems. New tests here: https://tbpl.mozilla.org/?tree=Try&rev=fb13891c0335
Made setTimeout be in the same scope as clearTimeout.
There was a minor problem solved in the previous patch. The try looks good: https://tbpl.mozilla.org/?tree=Try&rev=d60d164dd1d1 .
Good catch! Let's add checkin-need once most of the try results come in.
Try's taking forever and it looks OK so far, checkin-needed.
OS: Linux → All
Hardware: x86_64 → All
https://hg.mozilla.org/integration/fx-team/rev/afa010e8f62b In the future, keep in mind that your commit message should be summarizing what the patch is doing, not restating the problem :) https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #11) > https://hg.mozilla.org/integration/fx-team/rev/afa010e8f62b > > In the future, keep in mind that your commit message should be summarizing > what the patch is doing, not restating the problem :) > https://developer.mozilla.org/en-US/docs/Developer_Guide/ > Committing_Rules_and_Responsibilities#Checkin_comment I'll keep that in mind Ryan, thank you for the link :)
Assigning to Catalin since he handled bug 1041678 (and to balance load a bit).
QA Contact: cornel.ionce → catalin.varga
Verified fixed using latest Nightly, build ID: 20140821030201 on Mac OS X 10.9.4, Ubuntu 14.04 and Windows 7 64bit. When about:newtab is set to blank, the search bar can't be selected anymore neither in normal/private (by typing about:newtab) nor e10s windows.
You need to log in before you can comment on or make changes to this bug.