Page MenuHomePhabricator

Remove "Search Preferences"
ClosedPublic

Authored by epriestley on May 26 2016, 1:40 AM.
Tags
None
Referenced Files
F12826927: D15976.id.diff
Thu, Mar 28, 9:42 AM
Unknown Object (File)
Sun, Mar 24, 11:47 PM
Unknown Object (File)
Sun, Mar 24, 10:36 PM
Unknown Object (File)
Sun, Mar 24, 8:09 PM
Unknown Object (File)
Wed, Mar 20, 4:06 PM
Unknown Object (File)
Fri, Mar 1, 12:33 AM
Unknown Object (File)
Fri, Mar 1, 12:32 AM
Unknown Object (File)
Fri, Mar 1, 12:31 AM
Tokens
"Dislike" token, awarded by eadler.

Details

Summary

Ref T4103. This removes these options:

Screen Shot 2016-05-25 at 6.37.28 PM.png (211×749 px, 19 KB)

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
Branch
pref1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 12336
Build 15603: Run Core Tests
Build 15602: arc lint + arc unit

Event Timeline

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

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

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).

It's not there on windows:

pasted_file (190×522 px, 19 KB)

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:

all_controls.png (583×670 px, 86 KB)

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).

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 /.

/ 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 :(

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.