In about:permissions, Ctrl+f should focus the filter box instead of invoking the find bar

VERIFIED FIXED in Firefox 43

Status

()

Firefox
Preferences
--
enhancement
VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: Mardeg, Assigned: Steffen Wilberg)

Tracking

Trunk
Firefox 43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 verified)

Details

(Whiteboard: [testday-20110603], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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

7 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

7 years ago
Severity: normal → enhancement

Updated

5 years ago
Blocks: 821049
(Assignee)

Comment 2

3 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

3 years ago
Created 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?(margaret.leibovic)

Comment 4

3 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 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

3 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

3 years ago
Created attachment 8651943 [details]
MozReview Request: address review comments.

address review comments.
Attachment #8651943 - Flags: review?(jaws)
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+
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

3 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

3 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)
(Assignee)

Comment 13

3 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)
(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

3 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

3 years ago
Attachment #8651943 - Attachment is obsolete: true
(Assignee)

Comment 16

3 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 18

3 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 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

3 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
https://hg.mozilla.org/mozilla-central/rev/d6ea1920cbe2
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
QA Whiteboard: [good first verify]

Comment 22

2 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

2 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]
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
status-firefox43: fixed → verified
You need to log in before you can comment on or make changes to this bug.