Page MenuHomePhabricator

Remove "Search Preferences"
ClosedPublic

Authored by epriestley on May 26 2016, 1:40 AM.

Details

Summary

Ref T4103. This removes these options:

The jump nav option came from T916, when we had a separate jump nav on the home page. Essentially no one has ever been confused by the behavior of search or disabled this feature. Here are the stats for this install:

Total Users36656
Have Set Any Preference3084
Have Disabled Jump6
Are Not "Security Researchers"2
Any Account Activity0

The "/" option came in the same change, but the preference came from T989. This keystroke conflicts with a default Firefox keystroke. Almost no one cares about this either, but I count 6 real users who have disabled the behavior. I suspect the number of real users who use it may be smaller.

In Safari and Firefox, the "tab" key does the same thing.

In Chrome, the "tab" key does the same thing if PreferencesWeb Content"Pressing Tab highlights..." is disabled.

Upshot: jump nav is great, bulk of the change in T989 was clearly great, specific preferences that came out of it seem not-so-great and now is a good time to kill them as we head into T4103.

Test Plan
  • Grepped for removed constants.
  • Pressed "/".
  • Searched for T123.
  • Viewed settings.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley updated this revision to Diff 38461.May 26 2016, 1:40 AM
epriestley retitled this revision from to Remove "Search Preferences".
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad accepted this revision.May 26 2016, 2:28 AM
chad edited edge metadata.
This revision is now accepted and ready to land.May 26 2016, 2:28 AM
This revision was automatically updated to reflect the committed changes.
avivey added a subscriber: avivey.May 26 2016, 6:08 PM

hitting / stopped jumping to the quicksearch for me here :(
No errors in console (Chrome/windows).

epriestley added inline comments.May 26 2016, 6:11 PM
webroot/rsrc/js/core/behavior-keyboard-shortcuts.js
33–42

I removed the behavior explicitly, here. This didn't just remove the preference, it removed the behavior.

Try tab instead, or write a browser extension. "/" already has a default behavior in Firefox.

Can we at least add an explicit id to the field, to make browser extensions easier to write? tab already has a common behavior, "jump to next intractable element", I'm totally perplexed that it's doing anything else in FF.

Search is the first interactable element on the page, so pressing tab with nothing focused jumps to it.

In Firefox, "/" brings up "find on page", which is presumably what this keystroke was inspired by (?).

Maybe try something like this to select the element:

document.querySelector('.phabricator-main-menu-search-container input[name=query]');

For me, it's the ninth object (After all the header items). Not sure what determines the tab order (I have no "Pressing Tab" option in browser settings).

Here's what I see on OSX:

It's not there on windows:

I guess highlighting links/keyboard navigation isn't something OSX users think is important?

The default behavior of the OS and Safari/Firefox is to tab through only inputs which you can type text into (textarea/text inputs).

There's an accessibility setting to make it focus all controls:

Chrome's default behavior is to focus everything, which is nonstandard. Unchecking the preference makes it more consistent with the OS and with other browsers on OSX, so it makes some sense that maybe this is a Mac-only setting.

https://bugs.chromium.org/p/chromium/issues/detail?id=31177 is the chrome ticket; The "tab only sees things you can type into" is a OSX-ism, AFAICT; I know Windows is always "visit everything", and I guess (But don't know) that most linuxes would do that too (because linux prefers keyboard nav).

There may be a tab-order instruction to make the search field first? That will have the expected/desired behavior for everybody.

That will have the expected/desired behavior for everybody.

That would override OS behavior for Windows, Linux, and the default for Chrome. I don't want to hijack OS/browser behaviors (and then add options to undo the hijacking).

If we did try restore this I'd want to pick some new key which doesn't conflict with anything (looks like Firefox applies default behavior to ', too) and then just make that one unconditional and not add a preference for it (no one has ever run into issues with other global keybinds -- esc \, ?, ` -- other than / because of Firefox AFAIK).

avivey added a comment.EditedMay 26 2016, 7:04 PM

I think any sane (Non-console based) browser will leave most letter-keys unused. Vimium takes /, ?, j/k and other useful things, but it leaves q unused...

At least on Windows, I'd be surprised to see any non-modified, non-special key being used by a browser.

Yeah -- it's pretty weird to me that Firefox takes them, especially since Command-F already does almost exactly the same thing as /.

eadler added a subscriber: eadler.May 31 2016, 4:24 AM

/ is also a fairly common keyboard shortcut: gmail, feedly, facebook, twitter, and almost ever other website with keyboard shortcuts that I could find.

Having a jump-nav key was a boon. I just added a rule to vimium to stop listening to / on phabricator pages and was loving life... Is it likely we'll see a different key used to replace /? q looks like it would work, navigating around is much slower without /...

What about making it a user-settable option as part of the role profiles work, have it default to q but let me/others set it back to /?

This is a pretty major regression on Firefox+Gnome. I am no longer able to focus the search input with the keyboard. Tab doesn't work no matter how many times I tab I am only able to select text links. The search box does not appear to be anywhere in the tab order. OSX keyboard selection behavior is the weird edge case, not the norm.

Simply adding a tabindex=1 to the search input box seems to at least make the search box focusable, however, it's not as good as the global shortcut which works regardless of where focus is on the page.

other 'web applications such as slack and jira also use / for search. This seems like a regression :(

eadler awarded a token.Jun 1 2016, 3:52 PM

Most of my dev team and I miss '/' very much. All of us are Chrom(e|ium) users which requires Tab*8 to search with keyboard.

I use Tab to move around search results and documentation regularly.

As a Firefox user I have never experienced any trouble with the '/'-feature and was using it extensively. I would not mind any other key or combination being used for this.

In case anyone else is interested, I worked around this by installing a chrome extension called Shortcut Manager and added a little script to re-instate this functionality. If you want it, you can import the shortcut with the following from the settings pane of that plugin:

// ==UserScript==
// @ShortcutManager
// @name Search
// @namespace heQd49SJALoI
// @key /
// @include *phabricator*
// ==/UserScript==
queryBox = document.getElementsByName("query")[0]
queryBox.scrollIntoView()
queryBox.focus()

Hope that helps for anyone else landing here.