arni2033 Reporter
	Description • 8 years ago

>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR_1:
1. Open about:preferences#general
2. Press Ctrl+K

AR:  No visible action
ER:  Searchbar should be focused

	Cosmin Muntean [:cmuntean], Ecosystem QA (PTO until September 8th)
	Comment 1 • 8 years ago

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Firefox 50.1.0, Build ID: 20161208153507

I have managed to reproduce this issue on the latest Firefox (50.1.0) release and latest Nightly (53.0a1) build.

It seems that this issue is a regression. Using the mozregression tool I got to this results:
Last good revision: 3f135a8ead22 (2015-08-10)
First bad revision: 8cba870a352c (2015-08-11)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f135a8ead22&tochange=8cba870a352c

I suspect bug 1192606 which fixed the shortcut for search bar from "about:preferences#applications". Probably this fixed is also applied for the rest of the options from "about:preferences" and breaks the shortcut to the search bar from the browser tooblar.

	Steffen Wilberg Assignee
	Comment 2 • 7 years ago

Comment 1 is spot on.
The keyset defined in applications.xul is applied to the other pref panes as well (which it shouldn't).
Pref panes other than the current pane get "hidden" attributes. On the keyset, a "disabled" attribute needs to be set instead.

	Steffen Wilberg Assignee
	Comment 4 • 7 years ago

The patch re-enables the Ctrl+K shortcut to focus the Search box next to the URL bar - except for the Applications pane, where it focusses the search box in the Applications pref pane.

It also re-enables the Ctrl+F shortcut to open the find bar (except for the Applications pane, where it focusses the search box, like Ctrl+K above).
Note that this uncovers a bug with the new findbar highlighter controlled by the findbar.modalHighlight pref; it changes the text color to white, making it almost illegible, and doesn't add the yellow background color. The browser console shows this:

19:24:00.833 NS_ERROR_UNEXPECTED:  1 FinderHighlighter.jsm:1103
	_repaintHighlightAllMask resource://gre/modules/FinderHighlighter.jsm:1103:9
	_maybeCreateModalHighlightNodes resource://gre/modules/FinderHighlighter.jsm:1081:5
	update resource://gre/modules/FinderHighlighter.jsm:441:9
	_notify resource://gre/modules/Finder.jsm:118:5
	fastFind resource://gre/modules/Finder.jsm:193:5
	_find chrome://global/content/bindings/findbar.xml:1005:13
	onxblinput chrome://global/content/bindings/findbar.xml:82:9

I'd suggest to fix this bug and file a follow-up to fix the find bar highlighter in about:preferences. If that isn't fixed in time for release, we probably should disable ctrl+f in about:preferences.

	:Gijs (he/him)
	Comment 6 • 7 years ago

I won't be able to review this until Monday, but I don't think ctrl-f should be enabled on about:preferences because it more generally doesn't work well. Searching for any of the strings in the pref descriptions of the general tab fails completely, even on beta 51. I'd rather keep it disabled. X-ref bug 967517 and friends.

I expect the finder highlighter being broken is because it's using APIs that aren't available for XUL documents, but we should break more gracefully. Mike would know more about that.

	Steffen Wilberg Assignee
	Comment 8 • 7 years ago

Good point.

The updated patch disables ctrl+f in about:preferences (except the Applications pane). This works in all of the main pref panes.

Regarding the subdialogs:
In non-e10s mode, the disablefastfind attribute disables the findbar everywhere.
In e10s mode, this doesn't work because of bug 1094240. I didn't workaround that yet.

	:Gijs (he/him)
	Comment 9 • 7 years ago
mozreview-review

Comment on attachment 8826942 [details]
Bug 1326821: about:preferences - Ctrl+K shortcut doesn't focus searchbar.

https://reviewboard.mozilla.org/r/104744/#review105696

This looks sane to me.

	Pulsebot
	Comment 11 • 7 years ago

Pushed by steffen.wilberg@web.de:
https://hg.mozilla.org/integration/autoland/rev/60cd009c1148
about:preferences - Ctrl+K shortcut doesn't focus searchbar. r=Gijs

	Steffen Wilberg Assignee
	Comment 12 • 7 years ago

browser/components/preferences/in-content/tests/browser_bug731866.js gave the following error:
TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_bug731866.js | Applications elements should be visible (id=) -
I fixed this by excluding <keyset>s from the visibility check (because keysets are never visible).

	Phil Ringnalda (:philor)
	Comment 13 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/60cd009c1148

	Sylvestre Ledru [:Sylvestre]
	Comment 14 • 7 years ago

Is it something that we want to uplift?
At least 52 as it is going to be an esr

	Mike de Boer [:mikedeboer]
	Comment 15 • 7 years ago

(In reply to :Gijs from comment #6)
> I expect the finder highlighter being broken is because it's using APIs that
> aren't available for XUL documents, but we should break more gracefully.
> Mike would know more about that.

Yeah, I'd like to fix that - so Steffen, can you please file a bug for me in the Toolkit - Find Toolbar component?
IOW: it's supposed to work in chrome/ XUL documents too, so this is a bug I think.

	Gerry Chang [:gchang]
	Comment 16 • 7 years ago

We've built 51 RC. This is too late for 51. Mark 51 won't fix.

	Mike de Boer [:mikedeboer]
	Comment 17 • 7 years ago

To clarify: the findbar highlighting problem only affects Nightly, because the modal highlighting feature is disabled on all other channels. This bug can and should not block any uplift process.

	Mike de Boer [:mikedeboer]
	Comment 18 • 7 years ago

Comment on attachment 8826942 [details]
Bug 1326821: about:preferences - Ctrl+K shortcut doesn't focus searchbar.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1192606
[User impact if declined]: Ctrl+K doesn't focus the search bar inputbox from sections other than about:preferences#applications.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes, by me.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Because the patch is small and fixes a regression that is contained and easy to verify.
[String changes made/needed]: n/a.

	Mike de Boer [:mikedeboer]
	Comment 19 • 7 years ago

Verified on Nightly of 2017-01-18.

	Julien Cristau [:jcristau]
	Comment 20 • 7 years ago

Comment on attachment 8826942 [details]
Bug 1326821: about:preferences - Ctrl+K shortcut doesn't focus searchbar.

fix ctrl-k on about:preferences, aurora52+

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/8cb1602aea58

	Steffen Wilberg Assignee
	Comment 22 • 7 years ago

(In reply to Mike de Boer [:mikedeboer] from comment #15)
> Yeah, I'd like to fix that - so Steffen, can you please file a bug for me in
> the Toolkit - Find Toolbar component?
> IOW: it's supposed to work in chrome/ XUL documents too, so this is a bug I
> think.

Filed bug 1336750.