Closed Bug 1204836 Opened 9 years ago Closed 9 years ago

In about:home, make any searchable keypress focus the search box, and restore ctrl+f to invoke the find bar.

Categories

(Firefox :: Keyboard Navigation, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 44
Tracking Status
firefox42 --- unaffected
firefox43 + verified
firefox44 + verified

People

(Reporter: quicksaver, Assigned: steffen.wilberg)

References

Details

(Keywords: regression)

Attachments

(1 file)

I'm not convinced at all of the benefits of this behavior.

(In reply to Steffen Wilberg from bug 1195038 comment #6)
> One argument is indeed consistency: We use accel+f to focus the
> search/filter box in preferences (cookie viewer; passwords manager; I just
> fixed Applications; bug 1192540 fixes the regression in about:config; bug
> 661831 introduces that to about:permissions) and the Add-ons manager (to be
> fixed by bug 1195060). You might argue that most of those operate on the
> same page though (with the exception being the Add-ons manager, which
> searches on addons.mozilla.org), so this argument is rather weak.

I think that rather than "weak", this isn't a valid argument at all. Even the add-ons manager search box presents the results embedded in the page (even if it fetched them remotely), consistent with all the other examples that Ctrl+F "does something in-page". Whereas about:home's search box actually changes urls to display the results; this falls more in the dominion of a "search" than a "find".

(In reply to Steffen Wilberg from bug 1195038 comment #6)
> A stronger argument is usefulness: On about:home, there is nothing to "find"
> on the page itself: You can't search for the buttons on the bottom and hit
> enter to activate them, even after closing the find bar. So the only useful
> action is to search on the web.

A stronger argument yes. But consider this scenario:
- go to about:home
- surf away to another page (by clicking links often found in the small welcome message below the search box)
- open the find bar by pressing Ctrl+F
- go back to about:home

The find bar is now accessible and fully functional. Which begs the question, if the find bar can be used in about:home, why can't I open it via the usual method? A stronger consistency point would be to actually disable the find bar in about:home, since you can technically follow the same steps for the add-ons manager or about:config, but the findbar won't actually work there (maybe it should be hidden automatically there, but that's a whole other thing).

I'm not claiming that it doesn't have its usefulness as mentioned, just that it's not enough to completely change the browser's expected behavior on a specific page [1].

Moreover...
> But I actually like the idea of stealing accel+k to focus the search box of
> about:home and about:newtab instead of focussing the search bar
What happened to this?

Or better yet, why not make it a quick-find style instead, where any keypresses of searchable characters go directly to about:home's search box (like the google homepage)? This would leave all normal/expected shortcuts available, while still keeping the usefulness of about:home's search box more readily available.

[1] https://support.mozilla.org/en-US/kb/keyboard-shortcuts-perform-firefox-tasks-quickly#w_search
Steffen and Gijs, as assignee and reviewer on bug 1195038 I'd like your opinion on this, but I'd also like Philipp's opinion in particular who provided the ux-review.
Flags: needinfo?(philipp)
I don't have a sophisticated personal opinion; I never use about:home so I'm not a good commentator on its use. It does seem to me that there is little point in using find-in-page on about:home. I questioned the consistency in the bug and UX said that going ahead was the right thing, so I reviewed the code purely for purposes of "is this the right way to implement this". Philipp's opinion here is more important.
The find bar doesn't do anything useful for about:home.
You can't even use the find bar to highlight one of the buttons and press enter.

The possibility of invoking the find bar by navigating to a web page, opening the find bar, and back to about:home, could be considered a bug.

accel+k does already focus the search box of about:home if you remove the search bar from the location bar and menu. We could change that to always focus the search box, even if the search bar is present in the location bar.

But I really like your suggestion of making any keypress of searchable characters focus the search box, and made a patch to do just that (and restore accel+f to invoke the find bar).
Assignee: nobody → steffen.wilberg
Summary: Ctrl+F shouldn't focus about:home's search box → In about:home, make any searchable keypress focus the search box, and restore ctrl+f to invoke the find bar.
Bug 1204836: In about:home, make any searchable keypress focus the search box.
Attachment #8663429 - Flags: ui-review?(philipp)
I like it! Although it's probably suspicious of me to say that since I did suggest it. :p

I wonder if typing in the page should replace the text in the search box though instead of append to it; i.e. type "abc", click anywhere on the page to blur the search box, type "xyz", search box would have "xyz" instead of "abcxyz".

That's probably not a good idea since when searching we leave the page anyway, so for as long as we're in the page we probably want some continuance like this. Although one could argue the case when we go back to the page and start typing again, we'd probably want to start over then instead of appending. Well, it's just something that occurred to me so I thought I'd mention it at least.
Enter and Backspace could also follow through to the search box if it isn't empty.
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

Nice! The type-ahead behavior is definitely better and makes it unnecessary to change the ctrl-f behavior.
Appending is better since it's less destructive in the case where someone accidentally deselects the search box e.g. by touching the trackpad.
Flags: needinfo?(philipp)
Attachment #8663429 - Flags: ui-review?(philipp) → ui-review+
OFFTOPIC: hm, I remember feedback from some users (including me) that about:home familiarly takes focus from urlbar. That forced some of us to simply set about:blank as homepage. And now these changes...
I really hope there's a (privileged?) group of users which will appreciate all this time consuming work
(In reply to arni2033 from comment #9)
> OFFTOPIC: hm, I remember feedback from some users (including me) that
> about:home familiarly takes focus from urlbar. That forced some of us to
> simply set about:blank as homepage. And now these changes...

As you said, about:home already takes focus from urlbar, so I don't see how this type-ahead behavior would negatively affect that at all. When the page opens, focus is already on the search box, so this patch only really matters when the user actually does click somewhere on the page to blur the search box, which would remove focus from the urlbar anyway if that's what had focus.

FWIW about:newtab also focuses (or should? does for me) the urlbar by default. It's probably a more useful homepage to get the focus there when starting up, unless you don't care about the tiles feature at all.

> I really hope there's a (privileged?) group of users which will appreciate
> all this time consuming work

My general impression is that most end users don't really change their homepage, unless something does it for them. I have no facts to support that, but if it was necessary (which I don't think it is), I'm sure there is telemetry data on this; I just don't check it now myself because I don't know how to use the dashboards.
(In reply to Luís Miguel [:quicksaver] from comment #7)
> Enter and Backspace could also follow through to the search box if it isn't
> empty.

Making Enter focus the search box would prevent pressing Tab to focus the Downloads/History/etc. buttons and pressing enter to activate them.

Making Backspace focus the search box would override its default command: go back one page in history.
So I'd rather not change those.
Attachment #8663429 - Flags: review?(gijskruitbosch+bugs)
(In reply to Steffen Wilberg from comment #11)
> (In reply to Luís Miguel [:quicksaver] from comment #7)
> > Enter and Backspace could also follow through to the search box if it isn't
> > empty.
> 
> Making Enter focus the search box would prevent pressing Tab to focus the
> Downloads/History/etc. buttons and pressing enter to activate them.
> 
> Making Backspace focus the search box would override its default command: go
> back one page in history.
> So I'd rather not change those.

Good point! I suppose I could make the point of "enter could follow through if none of those buttons were selected", but I'm not sure about it since it'd be inconsistent behavior if backspace never follows through.

BTW, what about the spacebar, is it going to the search box? I haven't tested it with the try-build at the time. There's not a whole lot to scroll in about:home, do you feel it should still retain its default scroll function as well?
(In reply to Luís Miguel [:quicksaver] from comment #12)
> (In reply to Steffen Wilberg from comment #11)
> > (In reply to Luís Miguel [:quicksaver] from comment #7)
> > > Enter and Backspace could also follow through to the search box if it isn't
> > > empty.
> > 
> > Making Enter focus the search box would prevent pressing Tab to focus the
> > Downloads/History/etc. buttons and pressing enter to activate them.
> > 
> > Making Backspace focus the search box would override its default command: go
> > back one page in history.
> > So I'd rather not change those.
> 
> Good point! I suppose I could make the point of "enter could follow through
> if none of those buttons were selected", but I'm not sure about it since
> it'd be inconsistent behavior if backspace never follows through.
> 
> BTW, what about the spacebar, is it going to the search box? I haven't
> tested it with the try-build at the time. There's not a whole lot to scroll
> in about:home, do you feel it should still retain its default scroll
> function as well?

Space activates focused buttons on linux and Windows.
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

https://reviewboard.mozilla.org/r/19803/#review18391

::: browser/base/content/abouthome/aboutHome.js:64
(Diff revision 1)
> +  if (modifiers != 0 || ev.charCode == 0) // ignore Tab, Insert, PageUp, etc.
> +    return;

So this should make an exception for space (charCode = 32)

::: browser/base/content/test/general/browser_aboutHome.js:423
(Diff revision 1)
> -  desc: "Cmd+f should focus the search box in the page",
> +  desc: "Pressing any key should focus the search box in the page, and send the key to it",

It would be nice if you could add a test that tested that activating one of the existing buttons on the page with space still worked (on non-OSX)
Attachment #8663429 - Flags: review?(gijskruitbosch+bugs)
Attachment #8663429 - Attachment description: MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. → MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs
Attachment #8663429 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

https://reviewboard.mozilla.org/r/19803/#review18431

Thanks. This looks good from code inspection; apologies for the travel-induced delay in the review time here.

::: browser/base/content/test/general/browser_aboutHome.js:500
(Diff revisions 1 - 2)
> +    yield promiseTabLoadEvent(gBrowser.selectedTab, null, "load");

Nit: create this promise event before you trigger the navigation, and yield after.
Attachment #8663429 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs
I rewrote that test because bug 1205286 made the Sync button open the Sync preferences in a new tab, instead of loading about:accounts in the current tab.
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

Please have a look at the updated test, thanks.
Attachment #8663429 - Flags: review+ → review?(gijskruitbosch+bugs)
(In reply to Luís Miguel [:quicksaver] from comment 12)
Web pages that take your input and put/send it wherever they want - are considered as malicious, unless they have an option to disable that behavior (e.g. google.com's live search)

I suggest to make this "feature" opt-out via about:config, not "in your face". Or even opt-in (then get
exact data on how few users need it). This'll be the first feedback from users anyway, don't you think?

It's actually very strange how valuable features (e.g. bug 1119049, similar to this) introduced in other
browsers and regressions/bugs from previous millenium are ignored in favor of... bugs like this one. I'm
really stunned by this; thanks in advance for telling me why this is happening/where should I ask that
(In reply to arni2033 from comment #20)
> Web pages that take your input and put/send it wherever they want - are
> considered as malicious, unless they have an option to disable that behavior
> (e.g. google.com's live search)

about:home doesn't actually take or send the input anywhere until you actually press enter or click the search button. It's not live-search as in google, it's just facilitating the input in the search box, which is arguably the only input-related ability about:home even has.

> It's actually very strange how valuable features (e.g. bug 1119049, similar
> to this) introduced in other browsers and regressions/bugs from previous
> millenium are ignored in favor of... bugs like this one.

OOT: bug 1119049 is actually much different from this bug. Just because both deal with keyboard events doesn't mean they're similar. I'm guessing that hasn't been fixed yet because "Core :: Audio/Video - Playback" has 1426 open bugs listed as of right now; it's obviously and probably rightfully prioritized under others.
(In reply to Luís Miguel [:quicksaver] from comment 21)
Please don't use invalid (and long) arguments in response.
1) about:home Does send your input to other sites (and you can't determine to which ones by looking at it). Create new profile->open browser console->enable "Net"->disable anything else->type on homepage
2) I'm "guessing" that noone looked at that bug carefully. Please don't post answer if you're unsure
(In reply to arni2033 from comment #22)
> Please don't use invalid (and long) arguments in response.
I only do when I feel they're constructive of course. And I was the one who suggested this behavior, so I feel it's my job to defend it at least to an acceptable point. ;)

> 1) about:home Does send your input to other sites (and you can't determine
> to which ones by looking at it). Create new profile->open browser
> console->enable "Net"->disable anything else->type on homepage
If I'm seeing the same thing as you are, that is to provide search suggestions. You can disable that in Options -> Search -> Provide search suggestions

> 2) I'm "guessing" that noone looked at that bug carefully. Please don't post
> answer if you're unsure
(This is becoming more and more OOT, so these will be my last words on the matter. It's been categorized to the approximate component since it was first reported, and even recently re-categorized; it's been looked at properly.)

My tangent was meant to address your "why is this bug being worked on and not another" statement: they're unrelated bugs, and (lack of) development on one shouldn't prevent work on another.
(In reply to Luís Miguel [:quicksaver] from comment 23)
> You can disable that in Options -> Search -> Provide search suggestions
Surprisingly, I'm aware of that. But I won't disable them, because I need and use them. Though, I need them only when I _myself_ focus the input field and send my data, not when software decides it for me.
> This is becoming more and more OOT
I already said there's *no need in OT if you don't know the exact answer*. But I'll better clarify my view. These 2 bugs may differ in implementation, but for end user they are the same: applying an action to {chosen target w/o focus}. My question was not "why is this bug and not another" but "why non thought-through enhancements w/o propper checking fly to Release at supersonic speed while old valuable parity features AND old known bugs are ignored". It's a common tendency.
(In reply to arni2033 from comment #24)
> (In reply to Luís Miguel [:quicksaver] from comment 23)
> > You can disable that in Options -> Search -> Provide search suggestions
> Surprisingly, I'm aware of that. But I won't disable them, because I need
> and use them. Though, I need them only when I _myself_ focus the input field
> and send my data, not when software decides it for me.

FWIW I disagree completely. Currently, pressing a-z or any other equivalent key does nothing in about:home (exception being FAYT which was already covered in the Find in Page discussion). With this patch, the _action_ of pressing one of those keys is the user telling the browser to use them; it's not the browser deciding for itself to do it. I don't see any potentially unexpected behavior there. Note that shortcuts/operating-system-combinations/etc are unaffected of course!

Steffen/Gijs, I don't think so but is it worth it requesting another ux-review on this point?
(In reply to Luís Miguel [:quicksaver] from comment #25)
> With this patch, the _action_ of pressing one of those keys is the user telling the browser
> to use them; it's not the browser deciding for itself to do it
I knew you would say something like that, but that is true only if user who just installed the browser is informed of such behavior. Actually, I think this is it, the compromise!
Showing a tooltip/block of text with this info the 1st time user visits "enhanced" about:home would solve everything. I know Tour tooltips are obtrusive (bug 1197193), but this may be a good option here
(there's no tooltip code in the diff currently, I checked)
I strongly disagree.
about:home is a search page, plus some buttons for Downloads, Bookmarks, History, etc.
When you open about:home, its search box is focused, so you can search right away. You can start typing your search term without having to click to the search box first. This is the behavior even without this patch.

However, after you click one of the buttons, e.g. on the Downloads button, to check you downloads, focus gets lost. Even after you close the Downloads window, the Downloads button is still focused. So if you want to enter a search term, you now have to grab your mouse and click the search box (or click shift+tab three times).

With this patch, you can start typing your search term, and the first keystroke will focus the search box.
Note that this doesn't affect keyboard shortcuts with ctrl/cmd or alt, or non-printable keys like F1-F10, arrow keys etc.

I honestly can't think of a valid use case of using about:home, start to type and expect the text to _not_ appear in the search box. Instead, think of someone looking at the keyboard while typing a search term, then looking back at you screen just to notice that the search term appeared in the find bar instead of the search box, is annoyingly dumb.

By the way, the search results popup contains a "Change Search Settings" link at the bottom, which takes you to, well, the Search Settings, where you can choose the default search engine, install additional search engines, turn off search suggestions, etc.

P.S. I'm a volunteer. I choose to fix this bug because I care about not showing the find bar on pages where it is useless or doesn't even work.
While some parts of those comments are indeed OOT, please don't hide them as they address the concern about this patch inducing potential malicious behavior.

Removed obsolete flag from comments 21-24.
Weekend interruption to make a few notes:

- arni, as I've noted before, please be more respectful of folks who fix/triage bugs - especially volunteers, like Luís and Steffen. I appreciate all the bugs you file, and I would never think you file them to be nasty to others - it would be nice if you wouldn't imply that we prioritize bugs to be nasty (to you or anyone else), either.


- prioritization in bugzilla is hard. 1.2 million bugs is a lot, and even individual components like "Firefox :: Theme" have over 1000 open bugs in them. It's hard to prioritize such numbers, esp. considering how small a team we are (there's about under 25-35 people paid to work specifically on Firefox for desktop, and 25,000+ open bugs in the Firefox component alone, not counting Toolkit, Hello, etc. - and that number is increasing). And then there's all the stuff that's not filed (yet). :-)
- we are worse than we should be about managing/prioritizing bugs, on top of that being objectively difficult because numbers. We're working on being better. I'm happy to listen (email me!) if you want to make concrete suggestions about where we're failing with objective reasoning about that (see below). If you don't trust me to do well, contact Mike Hoye instead (who's recently done a bunch of work specifically to make our triaging/prioritization work better).

- age is the most useless criterion to decide what needs to get fixed. It's not relevant. I'm sure we'll get a security bug reported before the year is out (as a matter of maths & probability, it's practically certain) which we'll need to fix ASAP and uplift to all our supported branches, and equally there are old bugs like, say, bug 216931 (12 years old), which I don't see getting prioritized in the next 2-3 years (unless someone decides to revamp all the preferences, which we might do if/when we drop XUL). Anyway, rather than referring to age, make a case as to why the bug is serious and deserves attention, and/or why fixing it is easy (the latter being best done by submitting a patch - things that seem simple often aren't).
- everybody thinks bugs that they run into (I was sad when Brendan WONTFIXed prolog integration (bug 332432)!) are more important than bugs that they do not run into or worse, that they feel make things worse. All of us have to recognize that we are biased, and try to objectively look at how many people particular issues affect, and how that overlaps with which sets of users.


Now, about this bug:
- prefs, especially about:config prefs, are not the solution to every disagreement, especially not when they are about concerns which are already addressed by other prefs (like the homepage pref and the disabling of search suggestions pref in this case). If I could get away with removing half of about:config tomorrow, I would do it.
- extra UI that gets in the way of the "99%" case is likewise not a solution. Realistically, dialogs and tooltips don't get read by most users anyway, and I just don't think that there is a serious enough downside here to warrant not doing this, especially considering the default focus behaviour that you referred to in comment 9. ui-review was already done; I don't think we need another one.
Attachment #8663429 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

https://reviewboard.mozilla.org/r/19803/#review19065
https://hg.mozilla.org/integration/fx-team/rev/aebae265f1ff0f85cc9e2c4f051166f896beb440
Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r=gijs
https://hg.mozilla.org/mozilla-central/rev/aebae265f1ff
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Seems to be working great in today's nightly so far!

Will this be uplifted to aurora? I wouldn't think so, and if that's the case I can open a bug to backout bug 1195038 from aurora if necessary. I think a change in behavior in Fx43, only to have it changed back in Fx44 will be unnecessary and probably confusing.
Steffen, can you request aurora approval?
Flags: needinfo?(steffen.wilberg)
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

Approval Request Comment
[Feature/regressing bug #]: This is a follow-up to bug 1195038, which already is on Aurora. Bug 1195038 made accel+f focus the search box on about:home.
This bug reverted that and made any searchable keypress focus the search box instead.

[User impact if declined]: If declined, we should back out bug 1195038 on aurora, because we shouldn't introduce a keyboard shortcut, which is to be removed in the next version.
[Describe test coverage new/current, TreeHerder]: This is on Nightly since October 05. It added two new tests.
[Risks and why]: Only affects about:home, and only situations where the search box is not focused anymore (it is focused on page load). So pretty little, I guess.
[String/UUID change made/needed]: This deletes the string (the letter "f" for the keyboard shortcut) introduced by bug 1195038.
Flags: needinfo?(steffen.wilberg)
Attachment #8663429 - Flags: approval-mozilla-aurora?
Makes sense to me, and I like the idea of keeping the interface consistent from 42 to 44.
Since we are reverting some previous work in 43 I'll track this as a regression. 

I want to make sure arni2033 ends up with working keyboard shortcuts as well. I'll look around for a shortcuts and localization meta bug.
Comment on attachment 8663429 [details]
MozReview Request: Bug 1204836: In about:home, make any searchable keypress focus the search box. ui-review=philipp, r?gijs

Reverting new behavior in 43 after ux review. This keeps our keyboard navigation behavior more consistent. Approved for uplift to aurora.
Attachment #8663429 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
Whiteboard: [needs to land on aurora]
For the future please consider string removal as a "string change", since it creates unnecessary noise for localization. The best approach would be to land a patch that doesn't remove the string from .properties, and let the change ride the trains.
QA Whiteboard: [good first verify]
I have reproduced this bug on Nightly 43.0a1 (2015-09-15) on ubuntu 14.04 LTS, 32 bit!

The bug's fix is now verified on Latest Aurora 44.0a2!

Build ID: 20151202004004
User Agent: Mozilla/5.0 (X11; Linux i686; rv:44.0) Gecko/20100101 Firefox/44.0

[bugday-20151202]
I have reproduced this bug with Firefox Nightly 43.0a1 (Build ID: 20150915030232) on 
windows 8.1 64-bit .

Verified as fixed with Firefox Beta 43.0b9 (Build ID: 20151203163240)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0

Verified as fixed with Firefox Aurora 44.0a2 (Build ID: 20151210004006)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0

As this bug's fix is also verified on ubuntu (Comment 40), I am marking this as verified.
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify] → [good first verify][bugday-20151211]
Verified as fixed with Firefox Aurora 44.0a2 (Build ID: 20151210004006)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Just a note that this change broke bug 1237337, and will break other snippets we've developed which accept user input outside of the search box.
See Also: → 1237203
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: