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

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P3
normal
RESOLVED FIXED
a year ago
a month ago

People

(Reporter: epang, Assigned: mkaply)

Tracking

unspecified
Firefox 63
Points:
---

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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).
(Assignee)

Comment 1

a year ago
How do you define blank?

Comment 2

a year ago
In browser/base/content/browser.js, there is a function for that.

https://searchfox.org/mozilla-central/search?q=isTabEmpty&path=

Comment 3

a year ago
See bug1458534 for external links.
Blocks: 1394304
Priority: -- → P3

Comment 4

11 months ago
Any news about this?
Comment hidden (mozreview-request)

Comment 6

11 months ago
mozreview-review
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+

Updated

11 months ago
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 8

11 months ago
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

Comment 9

11 months ago
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.

Comment 10

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/aa2c71e82371
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
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.