Open Bug 1673588 Opened 4 years ago Updated 2 months ago

Change urlbar results so opening in a tab will open them in background, leaving the panel open

Categories

(Firefox :: Address Bar, enhancement, P2)

Desktop
All
enhancement
Points:
3

Tracking

()

Tracking Status
firefox82 --- affected
firefox83 --- affected
firefox84 --- affected

People

(Reporter: cbaica, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: papercut, Whiteboard: [snt-scrubbed][search-papercut])

Attachments

(1 file, 1 obsolete file)

Suggested severity

  • S3

Affected versions

  • Fx83.0b4
  • Fx84.0a1

Affected platforms

  • win 10 x64
  • ubuntu 18.04

Steps to reproduce

  1. Launch Firefox.
  2. Write something in the address bar.
  3. From the drop down list middle click a result.
  4. From the drop down list middle click a one-off engine.

Expected result

  • Behavior for the middle click should be similar:
  1. a new-tab is opened in the background, drop-down still displayed.
  2. a new-tab is opened in the background, drop-down still displayed.

Actual result
3. a new-tab is opened with the search result and focused on.
4. a new-tab is opened in the background. search is performed with the clicked engine.

Regression range

  • Not a recent regression. Will come back with a regression range ASAP.

Additional notes

  • It would make sense to have a uniform behavior in the drop-down. As a user would want to open multiple tabs with different search engines in the background, the same case could be made for opening multiple suggestions.

This was discussed today at the UX meeting, it was decided we'd like to follow the behavior of other browsers here, where results opened in tabs will open in a background tab and the urlbar panel stays open. Shift allows to invert the behavior so that we open in a foreground tab and close the panel.
It should be implemented behind a feature pref, because it's a behavioral change and we want to ensure we have a way back in case of issues.

Summary: There is an inconsistency between middle clicking a search suggestion and a one-off engine → Change urlbar results so opening in a tab will open them in background, leaving the panel open
Severity: normal → S4
Type: defect → enhancement
Priority: -- → P2

some discussion and possible issues in bug 1364415

See Also: → 1364415
Keywords: papercut
Points: --- → 3

I took a stab at this. Sometimes keeping a menu/panel open can trigger inconspicuous buggy side-effects, but so far I haven't noticed any. Feedback? (Unlike moz-review, I didn't see any way to set reviewer after submitting. Must it now be in in commit message?)

Flags: needinfo?(mak)

I'm sorry, I have a long queue of reviews, I'll come to this but it may take some time.

I left some feedback on the patch, feel free to just set reviewer. You can edit the changeset directly in Phabricator to add a reviewer after submitting.
I think overall you touched the right code points, but it needs a bit more work to be top notch.

Flags: needinfo?(mak)
Assignee: nobody → stayopenmenu
Status: NEW → ASSIGNED
Attachment #9196726 - Attachment description: Bug 1673588 - Open in background and keep panel open when opening in tabs. → commit Bug 1673588 - Respect browser.tabs.loadInBackground and keep panel open when opening in tabs in background.
Attachment #9196726 - Attachment description: commit Bug 1673588 - Respect browser.tabs.loadInBackground and keep panel open when opening in tabs in background. → Bug 1673588 - Respect browser.tabs.loadInBackground and keep panel open when opening in tabs in background.

Addressed Marco's feedback, found/fixed bug in original patch & tweaked tests (including enabling detection for that bug).
Have not done Try run yet.

Just wanted to say that I'm eager to see this fix implemented before next ESR (v.91).

See Also: → 1748115

Any particular reason the the patch wasn't revived and landed?

Someone asked me about this and a related issue (bug 1364415), I've been trying to track down the shortcut possibilities and various branching paths so I thought sharing this might be useful. I'm willing to work on either issue or both. @tawn, are you interested in rebasing your patch?

  1. When using a keyboard shortcut that opens a urlbar result in a background tab (e.g. Shift + middle mouse button), the urlbar results popup should remain open in the same way the bookmarks popup does when you open a bookmark in a background tab. At least, there should be a pref to make it stay open so users can open many results in a row.

    But what should happen to the actual results? What should happen to text in the urlbar, and what should happen to the search mode? Should all that stuff remain exactly as it was before we activate the result? Or should it be reset, but the popup kept open?

  2. There isn't any way to make "load in background" the default behavior for urlbar results, unlike for bookmarks.

  3. However, 2 is a multifaceted issue, because bookmarks have two prefs that control where they open:

    • browser.tabs.loadBookmarksInTabs
    • browser.tabs.loadBookmarksInBackground

    The first pref determines whether the bookmark loads in current or in tab by default, and the second pref determines whether the bookmark loads in tab or tabshifted in the event that the open behavior has been modified by a keyboard modifier or by the first pref. That's important here because urlbar results are something that normally open in current. So if we only added a pref to switch from tab to tabshifted that wouldn't affect the default behavior, that would only affect the middle click behavior, such that MMB opens in a background tab and Shift + MMB opens in an active tab.

  4. An issue with Ctrl + Enter is that it has its own behavior, at least provided browser.urlbar.ctrlCanonizesURLs is true (which it is by default). However, I don't see any reason why Ctrl + Shift + Enter can't open the result in a background tab, since it currently just implements the default behavior. That is, Ctrl + Enter canonizes the search string but Ctrl + Shift + Enter activates the result as normal. So as far as I can tell, it's basically a free shortcut. We could make it basically an alias for Shift + Alt + Enter.

    I'm inclined to use two prefs, just like the bookmarks prefs. One determines whether left click and Enter open the result in the current tab or in a new tab (this pref already exists: browser.urlbar.openintab); and the other determines whether the result opens in an active tab or a background tab. I don't really expect loadDivertedInBackground to affect urlbar results. I personally wouldn't want it affecting my urlbar, since they are very different kinds of affordance. Bookmarks have their own pref for that, and they're even more similar to links than urlbar results are.

    The implementation in tawn's patch adds a new pref that determines whether loadDivertedInBackground is respected. So it's basically the same as adding a third, independent pref. If we go with that experimental pref, and that pref is never going to be removed (I don't think it should be, since I want to open links in background but treat urlbar results differently) then we might as well just make the third pref control urlbar results behavior independently of loadDivertedInBackground, with a similar treatment as bookmarks get. That would yield the following shortcut possibilities:

    1. Default state (both prefs false)
      1. current = Left click or Enter
      2. new active tab = Ctrl + left click, Alt + Enter, or middle click
      3. new background tab = Ctrl + Shift + left click, Shift + Alt + Enter, or Shift + middle click
    2. browser.urlbar.openinbackground = true
      1. current = Left click or Enter
      2. new active tab = Ctrl + Shift + left click, Shift + Alt + Enter, or Shift + middle click
      3. new background tab = Ctrl + left click, Alt + Enter, or middle click
    3. browser.urlbar.openintab = true
      1. current = Ctrl + left click, Alt + Enter, or middle click
      2. new active tab = Left click or Enter
      3. new background tab = Ctrl + Shift + left click, Shift + Alt + Enter, or Shift + middle click
    4. browser.urlbar.openinbackground and browser.urlbar.openintab = true
      1. current = Ctrl + left click, Alt + Enter, or middle click
      2. new active tab = Ctrl + Shift + left click, Shift + Alt + Enter, or Shift + middle click
      3. new background tab = Left click or Enter

Does that sound about right?

Another open question is what to do about similar inputs in the searchbar and what happens to its popup. The SUMO page will also have to be updated I guess.

Edit: Actually, making the pref only respect loadDivertedInBackground leaves fewer options. It means if user has loadDivertedInBackground set to false, then toggling the experimental pref has no effect. Either the pref is not respected and the default behavior persists, or the pref is respected but it yields default behavior. Whereas having an independent pref similar to the bookmarks pref allows all possible configurations. It's also more consistent because we already have browser.urlbar.openintab, which is almost perfectly analogous to browser.tabs.loadBookmarksInTabs.

Flags: needinfo?(stayopenmenu)

A few additional observations:

  1. There's a long standing quirk that if where == tab we open a new tab just to set the search mode. It's already possible to make it happen, but it's made a lot more obvious with these changes since we can configure Firefox to make the default left click action = open a new background tab. And that new tab can be just a blank new tab with a search mode if 1) there's no search string or 2) one of the local search modes is clicked.
  2. We need to clear the selected row/button and search mode when activating an element of the opposite type.
  3. And we also ought to update the urlbarview when urlbar results are opened in background tabs, because when you open a history or top sites result, it's now an open tab. So if you were to close the urlbarview and reopen it and type in the same search string, you'd most likely see identical results but with an open tab result instead of a history or top sites result. So it would look nicer if it "transformed" into an open tab result immediately. This will take some investigation though because that's gonna be a lot of async activity.

@mak are you still interested in this enhancement? I've been working on a somewhat different patch that implements the same setting in both urlbar and searchbar, but with different prefs since they're differentiated by browser.urlbar.openintab and browser.search.openintab.

Flags: needinfo?(mak)
See Also: → 1753863

Bug was more complex than expected and as mentioned in Bug 1753863 comment #3, no one seemed to have time to review. I'd like to see this functionality in Fx and am just trying to be helpful. If someone else can do this better/sooner, feel free. If not, I'll try to get back to it later if someone can review.

Flags: needinfo?(stayopenmenu)

The following patch is waiting for review from an inactive reviewer:

ID Title Author Reviewer Status
D101540 Bug 1673588 - Respect browser.tabs.loadInBackground and keep panel open when opening in tabs in background. tawn mak: Back Aug 21, 2022

:tawn, could you please find another reviewer or abandon the patch if it is no longer relevant?

For more information, please visit auto_nag documentation.

Flags: needinfo?(stayopenmenu)

Cancelling needinfo set by bot: Comment #17 still applies. Patch was submitted years ago; mak being unavailable for a few weeks is irrelevant.

Flags: needinfo?(stayopenmenu)

This requires a person with time to follow the whole project/change and right now we don't have that.

Flags: needinfo?(mak)
Whiteboard: [snt-scrubbed][search-papercut]

the broken down work was happening in bug 1753863, the patch here is obsolete.

Depends on: 1753863
See Also: 1753863

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: stayopenmenu → nobody
Status: ASSIGNED → NEW

(In reply to Cheryl Phillips from comment #27)

Created attachment 9387745 [details]
same problem

Thanks from unanensul1987.

What same problem? And what is that attachment?

Attachment #9387745 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: