App tabs overlapping again

RESOLVED FIXED in Firefox 4.0b10

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: marcia, Assigned: dao)

Tracking

({regression})

Trunk
Firefox 4.0b10
regression
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [hardblocker])

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 480229 [details]
Screenshot of tab overlap

Yesterday while doing some testing on Mac using the 20100930 build, I snapped the attached screenshot: .  But I haven't yet been able to reproduce - I might have been coming out of Private Browsing mode. 

Bug 579869 has been fixed so in theory this problem should be gone.
(Reporter)

Comment 1

7 years ago
Created attachment 482586 [details]
Screenshot from latest nightly after restarting
(Assignee)

Updated

7 years ago
Keywords: regressionwindow-wanted
(Reporter)

Comment 2

7 years ago
I will need to try to see if I can reproduce this again as it has been intermittent. I have seen it on my Mac laptop but not using any of the QA lab machines.
(Reporter)

Comment 3

7 years ago
For me this seems to happen after session restore, and is most easily reproduced with Gmail pinned. I just was able to reproduce this with a clean profile.

STR:
1. Open a tab with Gmail in it. Pin it.
2. Open a tab with Google calendar. Don't pin it.
3. Restart Firefox.
4. Observe that the tabs overlap.

Updated

7 years ago
blocking2.0: --- → ?
Probably a dupe of Bug 579869
(Reporter)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 579869
(Assignee)

Updated

7 years ago
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 6

7 years ago
Bug 579869 was fixed months ago. This is a regression and we need a regression range.
Keywords: regression
(Reporter)

Comment 7

7 years ago
Paul: Are you running with any extensions?
(In reply to comment #7)
> Paul: Are you running with any extensions?

dom inspector 2.0.8, https everywhere 0.2.3.development.1,  flashblock 1.5.14.2
(Reporter)

Comment 9

7 years ago
I have the following installed:
*Instant Preview1falseinstant.preview@agadak.net
*Add-on Compatibility Reporter0.7falsecompatibility@addons.mozilla.org
*Firebug1.5.4falsefirebug@software.joehewitt.com
*NoScript2.0.4false{73a6fe31-595d-460b-a920-fcc0f8843232}
*Adblock Plus1.2.2false{d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d}

In the profile I can consistently repro the issue using today's nightly, even if I launch Minefield in safe mode.

I have tried with a clean profile but I cannot yet reproduce it on the latest trunk or on earlier builds.
This is not Mac specific, it happens on Windows too, see Bug 601930 which is the same AFAICS.

STR from bug 601930:
1. Start Minefield with new profile
2. Open 2 browser windows.
3. Open a AppTab and a regular tab in each window.
4. Alt > File > Exit, Choose "Save and Quit" on Quit Minefield dialog.
5. Start Browser again

Actual Results:
 AppTab overlaps "regular" tab on the Tab Bar in one of the restored windows.

Expected Results:
 Should not.

I will try to locate a regression range over the weekend.
Regression range on Windows:
works	2010-09-14-04	http://hg.mozilla.org/mozilla-central/rev/5588c9796f0b
broken	2010-09-15-07	http://hg.mozilla.org/mozilla-central/rev/0caec4ddff74

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5588c9796f0b&tochange=0caec4ddff74

This is using the STR of comment 10 on Windows 7, it would be helpful if someone with access to a Mac can verify if this is the same range, then we can be sure this and bug 601930 are the same.
(In reply to comment #11)
> Regression range on Windows:
> works    2010-09-14-04   
> http://hg.mozilla.org/mozilla-central/rev/5588c9796f0b
> broken    2010-09-15-07   
> http://hg.mozilla.org/mozilla-central/rev/0caec4ddff74
> 
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5588c9796f0b&tochange=0caec4ddff74

Seem to be the case for OS X too. Nothing in there seems very suspicious. The closest would be bug 593967. There was a TM merge in there too, but that doesn't seem likely.
Keywords: regressionwindow-wanted
OS: Mac OS X → All
Hardware: x86 → All
Duplicate of this bug: 601930
Summary: App tabs overlapping again on Mac → App tabs overlapping again
(Assignee)

Comment 14

7 years ago
Could somebody who's able to reproduce this try to find the first bad changeset using hg bisect?

Comment 15

7 years ago
STR of comment #0 of Bug 601930
Regression window:
Works:
http://hg.mozilla.org/mozilla-central/rev/017d16925aa1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100914 Firefox/4.0b7pre ID:20100915000003
Fails:
http://hg.mozilla.org/mozilla-central/rev/3077db085f3c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100915 Firefox/4.0b7pre ID:20100915021250
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=017d16925aa1&tochange=3077db085f3c

Comment 16

7 years ago
In adittion to comment 15,
Build from 875f1912a091: Fails
Build from 90d644e0930d: works

Regressed by
875f1912a091	Dão Gottwald — Bug 593967 - Add more elements into tabbrowser tabs for easier stylability. r=gavin
Blocks: 593967
(Assignee)

Comment 17

7 years ago
Thanks. Unfortunately, I don't see yet what the problem with the patch from bug 593967 might be. (Also, I still haven't seen this happen with various profiles on Win XP / 7 / Linux.)

Comment 18

7 years ago
(In reply to comment #17)
> Thanks. Unfortunately, I don't see yet what the problem with the patch from bug
> 593967 might be. (Also, I still haven't seen this happen with various profiles
> on Win XP / 7 / Linux.)

Umm I can 100% repro:
Windows7 Classic , Aero Basic and Aero.
WindowsXP Classic and Luna.
Ubuntu8.04 Human.

Comment 19

7 years ago
After restoring window, 
As a matter of course, execute the following code, then the position of the tab becomes normal.
document.getElementById("tabbrowser-tabs")._positionPinnedTabs();
(Assignee)

Comment 20

7 years ago
Created attachment 487534 [details] [diff] [review]
wild guess

Does this help?

Comment 21

7 years ago
(In reply to comment #20)
> Created attachment 487534 [details] [diff] [review]
> wild guess
> 
> Does this help?

Nothing changed.
"transitionend" event does not fire while restoring window.
However pinTab is called the number of times of the app tab while restoring window.

WORKAROUND:
      <method name="pinTab">
        <parameter name="aTab"/>
        <body><![CDATA[
          if (aTab.pinned)
            return;

          if (aTab.hidden)
            this.showTab(aTab);

          this.moveTabTo(aTab, this._numPinnedTabs);
          aTab.setAttribute("pinned", "true");
-         this.tabContainer._positionPinnedTabs();
+         //xxx width of aTab is not changed immediately
+         setTimeout(function(self){self.tabContainer._positionPinnedTabs();},0, this);
          this.tabContainer.adjustTabstrip();
(Assignee)

Updated

7 years ago
Attachment #487534 - Attachment is obsolete: true
I know it's hard going trying to track this down, but we need to understand and fix it.

Dao, is there anything else Alice can give you to help chase this? Any chance there's localstore strangeness in her profile, any chance your profile has stuff in it that is preventing you from reproducing the problem?
blocking2.0: ? → betaN+
fwiw, 100% reproducible here for me too on OSX & Win7, new & existing profiles. I don't see it during a normal session restore, but it always see it after restoring a recently closed window.

Dao - let me know if you need me to try anything if you still can't reproduce.
(Assignee)

Comment 24

7 years ago
I see it when restoring a recently closed window indeed.
Assignee: nobody → dao
Status: REOPENED → ASSIGNED
(Assignee)

Comment 25

7 years ago
Created attachment 488310 [details] [diff] [review]
hack to prevent the bug

For some reason the favicon doesn't take up any space initially, although width:16px is applied. This patch works around the issue but shouldn't be needed. Seems to be a layout bug so far.

I haven't tested the patch on OS X, the min-width might be off there.
(Assignee)

Comment 26

7 years ago
In _positionPinnedTabs, the computed CSS visibility of the favicon is "collapse" when the bug occurs, but nothing seems to be setting that...
As expected it's "visible" when the bug doesn't occur.
(Assignee)

Comment 27

7 years ago
It's not clear to me what was going on here, but bug 582950 appears to have fixed it.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Depends on: 582950
Resolution: --- → WORKSFORME
Target Milestone: --- → Firefox 4.0b8
(Assignee)

Comment 28

7 years ago
(In reply to comment #27)
> It's not clear to me what was going on here, but bug 582950 appears to have
> fixed it.

It hid the issue via bug 614290, which is fixed now, exposing this bug again.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Assignee)

Updated

7 years ago
Duplicate of this bug: 625854
(Assignee)

Updated

7 years ago
Assignee: dao → nobody
No longer depends on: 582950
Target Milestone: Firefox 4.0b8 → ---
(Assignee)

Comment 30

7 years ago
I figure this needs to be a hard blocker.
Whiteboard: [hardblocker]
(Assignee)

Comment 31

7 years ago
Created attachment 504166 [details] [diff] [review]
patch

This "fixes" the issue by not positioning app tabs when the tab bar doesn't overflow. This is really a workaround as far as this bug is concerned, but one we would want to apply anyway since it avoids doing unnecessary work.

Depends on the current patch in bug 617506.
Assignee: nobody → dao
Attachment #488310 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #504166 - Flags: review?(gavin.sharp)
Comment on attachment 504166 [details] [diff] [review]
patch

Stealing review. *yoink*

Looks fine, but one minor style this you may or may not want to make... _positionPinnedTabs is a bit hard to read through because there are so many things conditional on |doPosition|. I tried quickly refactoring so just split everything into a single |if (doPosition) { ... } else { ... }|. Seems a little easier to follow, at the cost of duplicating the (trivial) for-loop.

See http://pastebin.mozilla.org/949280

r+ either way.
Attachment #504166 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 33

7 years ago
http://hg.mozilla.org/mozilla-central/rev/5e3c0dc7434b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
(Assignee)

Updated

7 years ago
Duplicate of this bug: 626391

Comment 35

7 years ago
Is anyone else experienced this using Win32 Nightly build 20110117 Firefox/4.0b10pre?

Screenshots: http://i54.tinypic.com/2n1ryao.jpg
http://i54.tinypic.com/ajqno9.jpg

App Tab (Pin Tab) more than one tab then Unpin one of them and the other App Tabs become non-visible and part of the Unpined Tab is cut off, this happens using the default theme or with a Personas theme applied. Didn't happen on Win32 Nightly build 20110116 Firefox/4.010pre.

Maybe a regression since this bug landed: https://bugzilla.mozilla.org/show_bug.cgi?id=601228

Please confirm if you can...
Thanks... :D
(Assignee)

Updated

7 years ago
Duplicate of this bug: 608794
Duplicate of this bug: 626604
You need to log in before you can comment on or make changes to this bug.