Open Bug 320989 Opened 16 years ago Updated 2 years ago

Reuse last tab if blank

Categories

(Firefox :: Tabbed Browser, defect, P4)

defect

Tracking

()

ASSIGNED

People

(Reporter: mconnor, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

If we call addTab and the last tab is empty, use that tab instead.  Patch upcoming.
Attached patch foo (obsolete) — Splinter Review
This reuses the last tab if the last tab is blank and the tab to be added isn't another blank tab.
Attachment #206435 - Flags: review?(robert.bugzilla)
about:blank doesn't always mean the tab is empty.  It could be a page generated using DOM2.
Comment on attachment 206435 [details] [diff] [review]
foo

Is there a reasonable way of detecting a modified about:blank URL?
Attachment #206435 - Flags: review?(robert.bugzilla)
There are also timing issues using this method... when opening multiple pages in new non focused tabs a previously opened tab with a page that hasn't loaded yet is often over written by the next page to be opened.
hmm, right, last time around I hit that, and I fixed that, but I lost the patch when OS X ate itself.
(In reply to comment #3)
> (From update of attachment 206435 [details] [diff] [review] [edit])
> Is there a reasonable way of detecting a modified about:blank URL?
> 

Simple check seems to be to check that body has no child nodes. Wouldn't catch everything though.
(In reply to comment #6)
> Simple check seems to be to check that body has no child nodes. Wouldn't catch
> everything though.

A slightly more elaborate check would be to fetch the document source as a string and compare it to a XMLHttpRequest DOM "string" for the true about:config document. I might be missing something, but I imagine that would be fairly reliable and non-destructive.
Comment on attachment 206435 [details] [diff] [review]
foo

>+            if (aURI != "about:blank") {

Why is about:blank special cased? What this implies to me is that when a group of tabs is opened if the first of the set is blank, the last _isn't_ reused and another blank one added. Is this to make sure that Ctrl+T still works? Something more tailored to that case would seem more appropriate if so.

>+                if (aPostData === undefined)
>+                  aPostData = null;

Isn't this coercion implicit? If not, why not change all the callers to make the API tighter, rather than add code bloat here?
>+              var browserForLastTab = this.getBrowserAtIndex(this.mTabContainer.childNodes.length - 1);

use this.mTabs instead of this.mTabContainer.childNodes
(In reply to comment #5)
> hmm, right, last time around I hit that, and I fixed that, but I lost the patch
> when OS X ate itself.
> 

Would checking nsiwebprogresslistener.isLoadingDocument cover that?
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → Firefox 2 beta1
*** Bug 302352 has been marked as a duplicate of this bug. ***
*** Bug 344381 has been marked as a duplicate of this bug. ***
Duplicate of this bug: 411478
Anyone interested in reviving this patch?
Assignee: mconnor → nobody
Status: ASSIGNED → NEW
Duplicate of this bug: 421279
Duplicate of this bug: 424516
Please change target milestone - current is no longer adequate.
Putting as Future for now, until we get something on this.
Target Milestone: Firefox 2 beta1 → Future
Assignee: nobody → dtownsend
Target Milestone: Future → ---
The new tab stuff will likely make this bug invalid.
Assignee: dtownsend → nobody
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Attachment #206435 - Attachment is obsolete: true
Attachment #378958 - Flags: review?(gavin.sharp)
Blocks: 344381
Status: NEW → ASSIGNED
Attached patch patchSplinter Review
A number of sessionrestore tests also needed attention. This passes on the tryserver.
Attachment #378958 - Attachment is obsolete: true
Attachment #379530 - Flags: review?(gavin.sharp)
Attachment #378958 - Flags: review?(gavin.sharp)
Blocks: 573580
Blocks: 581242
Blocks: 582906
No longer blocks: 582906
dao - does this still apply?  If so, feel free to point this request at me.
Comment on attachment 379530 [details] [diff] [review]
patch

This no longer applies cleanly.
Attachment #379530 - Flags: review?(gavin.sharp)
Duplicate of this bug: 1197982
Duplicate of this bug: 1383375
Duplicate of this bug: 1474188
Duplicate of this bug: 1502836
Duplicate of this bug: 1458534
Blocks: 1442843
See Also: → 1498093
Just the other day I asked about this exact issue in forums https://support.mozilla.org/en-US/questions/1240609 and after unsatisfying reply decided to open a new bug/feature request... just to find out that there is already one.

So a small +1, this is quite annoying when you have window with single, blank tab (ideally without history) and opening link from external tab open new tab next to this one.
Duplicate of this bug: 1539927

I didn't have this problem for many years until old add-ons were killed (which probably had their own workaround for this issue). Now i have switched to Firefox Quantum and webextensions and have to deal with this annoyance from 14 years ago. My own bug report has been closed as a duplicate of this. Though there is a number of similar reports still open. Not sure if this super old report with outdated comments and patches should be kept alive and untouched for another 14 years (as it seems). The thing is that in some cases Firefox reuses the blank tab and in some not. So this bug report is too broad and is not representing the current state. E.g.

Sometimes add-ons can reuse last blank tab, sometimes don't https://bugzilla.mozilla.org/show_bug.cgi?id=1539927
It reuses blank tab when opening a bookmark, but not when opening a few at once https://bugzilla.mozilla.org/show_bug.cgi?id=1498093
It is not reusing a blank tab when opening links from external application https://bugzilla.mozilla.org/show_bug.cgi?id=1502836

You need to log in before you can comment on or make changes to this bug.