When browser.search.openintab is true, reuse current tab if it is blank

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P3
normal
RESOLVED FIXED
Last year
3 months ago

People

(Reporter: epang, Assigned: mkaply)

Tracking

unspecified
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

To remain consistent with the prefs browser.urlbar.openintab and browser.tabs.loadBookmarksInTabs we should also update browser.search.openintab to reuse current new tab (if it's the active tab).
How do you define blank?
In browser/base/content/browser.js, there is a function for that.

https://searchfox.org/mozilla-central/search?q=isTabEmpty&path=
See bug1458534 for external links.
Blocks: 1394304
Priority: -- → P3
Any news about this?
Comment on attachment 8990150 [details]
Bug 1455326 - Don't open search in new tab if current tab is blank.

https://reviewboard.mozilla.org/r/255152/#review262052

Thanks Mike!

::: browser/components/search/content/search.xml:286
(Diff revision 1)
>                where += "-background";
>            } else {
>              var newTabPref = Services.prefs.getBoolPref("browser.search.openintab");
>              if (((aEvent instanceof KeyboardEvent) && aEvent.altKey) ^ newTabPref)
>                where = "tab";
> +            if (where == "tab" && isTabEmpty(gBrowser.selectedTab)) {

This should get the job done, but you could write it a little more simply IMO:

  if (((aEvent instanceof KeyboardEvent && aEvent.altKey) ^ newTabPref) &&
      !isTabEmpty(gBrowser.selectedTab)) {
    where = "tab";
  }

Since `where` is "current" by default.  Writing it this way also emphasizes that this is connected to the newTabPref and not e.g. the mouse check right below this.

But up to you, r+ either way

::: browser/components/search/content/search.xml:1791
(Diff revision 1)
>              }
>            } else {
>              var newTabPref = Services.prefs.getBoolPref("browser.search.openintab");
>              if (((aEvent instanceof KeyboardEvent) && aEvent.altKey) ^ newTabPref)
>                where = "tab";
> +            if (where == "tab" && isTabEmpty(gBrowser.selectedTab)) {

Here too of course
Attachment #8990150 - Flags: review?(adw) → review+
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/aa2c71e82371
Don't open search in new tab if current tab is blank. r=adw
Thanks for this, Mike!
Just leaving this here, as it's the last piece missing from this "puzzle" with this and bug1417133 being fixed: bug1458534.
https://hg.mozilla.org/mozilla-central/rev/aa2c71e82371
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Duplicate of this bug: 1363837
Duplicate of this bug: 344381
You need to log in before you can comment on or make changes to this bug.