Closed Bug 609700 Opened 14 years ago Closed 14 years ago

Shift-clicking the back button or middle-clicking it with browser.tabs.opentabfor.middleclick=false opens an invisible tab

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 4.0b8
Tracking Flags:
Tracking Status
blocking2.0 --- final+

People

(Reporter: mozilla, Assigned: dao)

References

Details

Attachments

(1 file)

patch
(deleted), patch
Gavin
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101013 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101013 Firefox/4.0b8pre

With no tabs visible (autohide preference true) and browser.tabs.opentabfor.middleclick set to false, middle clicking an active back or forward button opens the relevant page in a tab that is not visible in the tab bar.

Reproducible: Always

Steps to Reproduce:
1. Set browser.tabs.autohide true and browser.tabs.opentabfor.middleclick false
2. Open a window containing a single tab so that the tabbar auto-hides
3. Browse from one page to another to activate the back button
4. Middle click on the back button
Actual Results:  
A single tab appears in the tab bar, where no tab was showing before, containing the current page.  If this tab is then closed, the page from the back button appears and the tabbar auto-hides.

Expected Results:  
The previous page should open in a new window, just like middle clicking on a link or bookmark.  New tabs should not appear with the browser.tabs.opentabfor.middleclick preference set to false.  The tabbar should not show just a single tab when the browser.tabs.autohide preference is set to true.

This behaviour was presumably introduced by bug 529240, but it seems a bit too old to be reopening.
Regression window:
Works;
http://hg.mozilla.org/mozilla-central/rev/73ab2c3c5ad9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100911 Firefox/4.0b6pre ID:20100911041541
Fails:
http://hg.mozilla.org/mozilla-central/rev/c4c9b356c106
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100910 Firefox/4.0b6pre ID:20100911034627
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=73ab2c3c5ad9&tochange=c4c9b356c106
Canditate:
dfbf5d853b89	Klaas Heidstra — Bug 448546 - When middle-clicking back/forward/reload, the new tab should inherit history from the tab that opened it (using duplicateTab). r=dao, ui-r+a=beltzner
Blocks: 448546
Status: UNCONFIRMED → NEW
Component: General → Tabbed Browser
Ever confirmed: true
OS: Linux → All
QA Contact: general → tabbed.browser
Target Milestone: --- → Firefox 4.0
Version: unspecified → Trunk
This called by middle click
function duplicateTabIn(aTab, where, historyIndex) {
  let newTab = gBrowser.duplicateTab(aTab);

  // Go to index if it's provided, fallback to loadURI if there's no history.
  if (historyIndex != null) {
    try {
      gBrowser.getBrowserForTab(newTab).gotoIndex(historyIndex);
    }
    catch (ex) {
      let sessionHistory = aTab.linkedBrowser.sessionHistory;
      let entry = sessionHistory.getEntryAtIndex(historyIndex, false);
      let fallbackUrl = entry.URI.spec;
      gBrowser.getBrowserForTab(newTab).loadURI(fallbackUrl);
    }
  }

  var loadInBackground =
    getBoolPref("browser.tabs.loadBookmarksInBackground", false);

  switch (where) {
    case "window":
      gBrowser.hideTab(newTab);
      gBrowser.replaceTabWithWindow(newTab);
      break;


Nothing happens here, because this.visibleTabs.length==1 now (Original tab and hidden duplicated tab).

      <method name="replaceTabWithWindow">
        <parameter name="aTab"/>
        <body>
          <![CDATA[
            if (this.visibleTabs.length == 1)
              return null;
(In reply to comment #2)
> Nothing happens here, because this.visibleTabs.length==1 now (Original tab and
> hidden duplicated tab).
> 
>       <method name="replaceTabWithWindow">
>         <parameter name="aTab"/>
>         <body>
>           <![CDATA[
>             if (this.visibleTabs.length == 1)
>               return null;

This is technically the fault of bug 582116. We should keep using this.tabs.length here. Alice or Klaas, does either of you want the write the patch?
Blocks: 582116
Dão san ,please
Summary: Middle clicking the back button opens an invisible tab → Shift-clicking the back button or middle-clicking it with browser.tabs.opentabfor.middleclick=false opens an invisible tab
Attached patch patch (deleted) — — Splinter Review 
Assignee: nobody → dao
Status: NEW → ASSIGNED

        Attachment #488449 -
        Flags: review?(gavin.sharp)

    
    

    
  
Comment on attachment 488449 [details] [diff] [review]
patch

>diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml

>+            return window.openDialog(getBrowserURL(), "", "dialog=no,all", aTab);

nit: use "_blank" to be consistent with other places where we open a new window

        Attachment #488449 -
        Flags: review?(gavin.sharp) → review+
blocking2.0: --- → final+

    
    

    
  
Is "_blank" beneficial in any way? If not, I'd rather fix the other places.

    
    

    
  
_blank is a special value that informs Firefox explicitly to open a new unnamed window rather than potentially using an existing one.  Use it if that is what you want.

    
    

    
  
"" and "_blank" are effectively the same, given http://hg.mozilla.org/mozilla-central/annotate/415bf458e0df/embedding/components/windowwatcher/src/nsWindowWatcher.cpp#l788 . I prefer "_blank" because it's behavior is well known and obvious.

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/188416ff6ff0
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: Firefox 4.0 → Firefox 4.0b8

    
    

    
  
11/11/10 nightly.  It kind of works :)  No more invisible tabs.  New tab or window opens as appropriate with the right page.  But it flashes the tab bar on and off for no reason?

    
    

    
  
(In reply to comment #11)
> 11/11/10 nightly.  It kind of works :)  No more invisible tabs.  New tab or
> window opens as appropriate with the right page.  But it flashes the tab bar on
> and off for no reason?

It's because there's temporarily a second tab (a hidden one) and browser.tabs.autoHide doesn't differentiate between hidden and visible tabs.

    
    

    
  
(In reply to comment #12)
> It's because there's temporarily a second tab (a hidden one) and
> browser.tabs.autoHide doesn't differentiate between hidden and visible tabs.

Looks like we should address this in a new bug. Flashing the tabbar doesn't sound like the final solution. Is there already a bug on file?

    
    

    
  
I raised bug 613580 for the tab-flashing behaviour.

    
    

    
  
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101204 Firefox/4.0b8pre ID:20101204030328
Status: RESOLVED → VERIFIED
Flags: in-litmus-

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: