Closed
Bug 661831
Opened 13 years ago
Closed 9 years ago
In about:permissions, Ctrl+f should focus the filter box instead of invoking the find bar
Categories
(Firefox :: Settings UI, enhancement)
Firefox
Settings UI
Tracking
()
VERIFIED
FIXED
Firefox 43
Tracking | Status | |
---|---|---|
firefox43 | --- | verified |
People
(Reporter: mardeg, Assigned: steffen.wilberg)
References
()
Details
(Whiteboard: [testday-20110603])
Attachments
(1 file, 1 obsolete file)
Aleksej suggested this be filed as a new bug dependent on bug 661823, and better to do this than nothing. The main issue will be when the Find bar already exists, should Ctrl F close it and focus the "Search Sites" box in the one go?
Comment 1•13 years ago
|
||
If possible, this might also be good for about:addons.
OS: Windows XP → All
Hardware: x86 → All
Summary: Doing Ctrl F in about:permissions should focus the "Search Sites" box → Doing Ctrl+F in about:permissions (and about:addons?) should focus the "Search Sites" box
Updated•13 years ago
|
Severity: normal → enhancement
Assignee | ||
Comment 2•9 years ago
|
||
I filed bug 1195060 to fix the Add-ons manager, so this bug should only be about about:permissions.
Assignee: nobody → steffen.wilberg
Summary: Doing Ctrl+F in about:permissions (and about:addons?) should focus the "Search Sites" box → In about:permissions, Ctrl+f should focus the filter box instead of invoking the find bar
Assignee | ||
Comment 3•9 years ago
|
||
Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar.
Attachment #8648458 -
Flags: review?(margaret.leibovic)
Comment 4•9 years ago
|
||
Comment on attachment 8648458 [details] MozReview Request: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. Redirecting to a desktop engineer.
Attachment #8648458 -
Flags: review?(margaret.leibovic) → review?(jaws)
Comment 5•9 years ago
|
||
Comment on attachment 8648458 [details] MozReview Request: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. https://reviewboard.mozilla.org/r/16221/#review14549 ::: browser/components/preferences/aboutPermissions.js:889 (Diff revision 1) > + let filterBox = document.getElementById("sites-filter"); Can you add a sitesFilter member variable and initialize it in init(), similar to what is done for sitesList ? Then update the other two places that call getElementById("sites-filter") to use the member variable?
Attachment #8648458 -
Flags: review?(jaws)
Assignee | ||
Comment 6•9 years ago
|
||
Comment on attachment 8648458 [details] MozReview Request: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar.
Assignee | ||
Comment 7•9 years ago
|
||
address review comments.
Attachment #8651943 -
Flags: review?(jaws)
Comment 8•9 years ago
|
||
Comment on attachment 8651943 [details] MozReview Request: address review comments. https://reviewboard.mozilla.org/r/17035/#review15173 Ship It!
Attachment #8651943 -
Flags: review?(jaws) → review+
Comment 9•9 years ago
|
||
Hey Steffen, It looks like there was separate commits for the reviews here, rather than appending to the previous review request. Can you post a patch that merges the two commits here? We can mark that one for checkin-needed.
Flags: needinfo?(steffen.wilberg)
Assignee | ||
Comment 10•9 years ago
|
||
url: https://hg.mozilla.org/integration/fx-team/rev/5b6fdfc822e9d18614035d0ce4962ac63eb842b6 changeset: 5b6fdfc822e9d18614035d0ce4962ac63eb842b6 user: Steffen Wilberg <steffen.wilberg@web.de> date: Mon Aug 24 22:18:54 2015 +0200 description: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. r=jaws
Assignee | ||
Comment 11•9 years ago
|
||
Yeah, I just started using hg bookmarks and MozReview and am not sure about the best workflow yet. Anyway, I rolled the two commits into one and pushed that, see comment 10.
Flags: needinfo?(steffen.wilberg)
Comment 12•9 years ago
|
||
Backed out for browser_zbug569342.js failures. https://treeherder.mozilla.org/logviewer.html#?job_id=4354991&repo=fx-team https://hg.mozilla.org/integration/fx-team/rev/979bb826f040
Assignee | ||
Comment 13•9 years ago
|
||
uncaught exception - TypeError: this.sitesFilter is null at chrome://browser/content/preferences/aboutPermissions.js:891 That's this line: this.sitesFilter.focus(); Looks like a race between the init function of the page, which sets this.sitesFilter, and the test, which synthesizes ctrl+f. Should I add if (!this.sitesFilter) this.sitesFilter = document.getElementById("sites-filter"); to focusFilterBox, to make that stupid test happy?
Flags: needinfo?(jaws)
Comment 14•9 years ago
|
||
(In reply to Steffen Wilberg from comment #13) > uncaught exception - TypeError: this.sitesFilter is null at > chrome://browser/content/preferences/aboutPermissions.js:891 > > That's this line: > this.sitesFilter.focus(); > > Looks like a race between the init function of the page, which sets > this.sitesFilter, and the test, which synthesizes ctrl+f. > > Should I add > if (!this.sitesFilter) > this.sitesFilter = document.getElementById("sites-filter"); > to focusFilterBox, to make that stupid test happy? It would be better to make siteFilter a getter that calls getElementById and overwrites the getter with the result of the getElementById call.
Flags: needinfo?(jaws)
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8648458 [details] MozReview Request: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar.
Assignee | ||
Updated•9 years ago
|
Attachment #8651943 -
Attachment is obsolete: true
Assignee | ||
Comment 16•9 years ago
|
||
https://reviewboard.mozilla.org/r/16221/#review15203 ::: browser/components/preferences/aboutPermissions.js:361 (Diff revision 3) > + get sitesFilter () { > + let sitesFilter = document.getElementById("sites-filter"); > + > + // Cache this value, overwrite the getter. > + Object.defineProperty(this, "sitesFilter", {value: sitesFilter, enumerable: true}); > + > + return sitesFilter; > + }, stolen from http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/cells.js#37
Assignee | ||
Comment 17•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f0af8f1f854
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8648458 [details] MozReview Request: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar.
Attachment #8648458 -
Flags: review?(jaws)
Comment 19•9 years ago
|
||
Comment on attachment 8648458 [details] MozReview Request: Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. https://reviewboard.mozilla.org/r/16221/#review15249 ::: browser/components/preferences/aboutPermissions.js:365 (Diff revision 3) > + Object.defineProperty(this, "sitesFilter", {value: sitesFilter, enumerable: true}); This can be simplified to: get sitesFilter() { delete this.sitesFilter; return this.sitesFilter = document.getElementById("sites-filter"); } This is similar to http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-places.js#1208
Attachment #8648458 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 20•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d6ea1920cbe2512c9b03ae3f5dd6846badd3f858 Bug 661831 - In about:permissions, make Ctrl+f focus the filter box instead of invoking the find bar. r=jaws
Comment 21•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d6ea1920cbe2
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Updated•9 years ago
|
QA Whiteboard: [good first verify]
Comment 22•9 years ago
|
||
Successfully reproduce the bug on Name Firefox Version 42.0 Build ID 20151029151421 User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:42.0) Gecko/20100101 Firefox/42.0 The fix works for me on Name Firefox Version 43.0b1 Build ID 20151103023037 User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:43.0) Gecko/20100101 Firefox/43.0
Comment 23•9 years ago
|
||
My bad, I forgot to update to latest beta. Fix also works in Name Firefox Version 43.0b4 Build ID 20151116155110 User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:43.0) Gecko/20100101 Firefox/43.0
QA Whiteboard: [good first verify] → [good first verify][bugday-20151118]
Comment 24•9 years ago
|
||
I have successfully reproduced this bug on Firefox Nightly 7.0a1 (2011-06-03) on Windows 7, 64 bit! This bug's fix is now verified on Latest 43.0 Beta 4. Build ID 20151116155110 User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0 As it has been verified on MAC as per comment 23, Changing the status!
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•