Closed
Bug 1455326
Opened 6 years ago
Closed 5 years ago
When browser.search.openintab is true, reuse current tab if it is blank
Categories
(Firefox :: Search, enhancement, P3)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: epang, Assigned: mkaply)
References
Details
Attachments
(1 file)
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•6 years ago
|
||
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.
Comment hidden (mozreview-request) |
Comment 6•5 years 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•5 years ago
|
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
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.
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/aa2c71e82371
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in
before you can comment on or make changes to this bug.
Description
•