967917 - Enable browser_tabbar_big_widgets.js across platforms

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

STR:

1) Create a tall widget - either by mucking with DOM inspector, or perhaps install an add-on that provides one.
2) Enter customize mode
3) Make sure the titlebar is not being shown
4) Drag the tall widget into the tabs toolbar
5) Exit customize mode

ER:

The tabstrip and titlebar overlap should do its best to accommodate the tall widget.

AR:

While the tabstrip gets taller, the titlebar behind it stays at its original size, which looks really really glitchy.

We had a sort-of fix for this, but it got backed out for the (arguably more important) bug 930094.

With bug 930094 closed, we should see if there are alternatives or solutions to this that doesn't regress bug 930094.

We should also re-enable the browser_tabbar_big_widgets.js test for OS X when we solve this, since it was disabled to land bug 930094.

Comment 1

[Dão Gottwald [:dao]]

Why is it necessary for bug 930094 to regress this?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Dão Gottwald [:dao] from comment #1)
> Why is it necessary for bug 930094 to regress this?

We felt that it's far more likely that people will experience the UI glitches by disabling tabs in titlebar than the glitches caused by dragging tall widgets into the tabstrip. So it's a stopgap, which hopefully buys us time to solve this one (again) without regressing bug 930094.

Comment 3

[Dão Gottwald [:dao]]

Why did bug 930094 end up "optimizing" this in the first place? Bug 930094 comment 81 just wrongly says we were doing unnecessary calculations, but it doesn't explain why removing them would be the right thing to do even for the purpose of that bug.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

(In reply to Dão Gottwald [:dao] from comment #3)
> Why did bug 930094 end up "optimizing" this in the first place? Bug 930094
> comment 81 just wrongly says we were doing unnecessary calculations, but it
> doesn't explain why removing them would be the right thing to do even for
> the purpose of that bug.

I don't think "optimizing" is the right word. More like simplifying, as a way to make it so that the UI didn't glitch when enabling and disabling tabs in titlebar.

You're right that I was wrong to say that the calculations were unnecessary, but I was correct when I said that those calculations were contributing heavily to the tabs in titlebar glitching. As I said, removing them was a stopgap to fix the more likely scenario of a user disabling tabs in titlebar.

Comment 5

[Dão Gottwald [:dao]]

How about we revert those wrong changes and back out bug 962677?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

That doesn't make a whole lot of sense - both bug 962677 and bug 930094 were deemed high priority bugs, and you yourself marked this as a P3 (which, while important, has less priority than bug 962677 and bug 930094).

I think the tradeoff is sound - we killed two higher priority bugs, and re-opened a lower priority bug, which users are less likely to encounter.

Comment 7

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #6)
> I think the tradeoff is sound - we killed two higher priority bugs, and
> re-opened a lower priority bug, which users are less likely to encounter.

FWIW, I agree with Mike here. I don't like regressions any more than the next guy, but the issue of people putting widgets in the tabstoolbar that are more than 30 pixels tall has always seemed very theoretical to me in the first place.

Comment 8

[Dão Gottwald [:dao]]

Bug 962677 is marked P-. It blocks a P2 meta bug, but surely not every bug in that dependency list can count as a P2 on its own.

I marked this P3 because bug 884492 was P3.

Comment 9

[Dão Gottwald [:dao]]

And incidentally, bug 962677 caused another P2 regression (bug 967220)... Seems like a perfect back-out candidate.

Comment 10

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #9)
> And incidentally, bug 962677 caused another P2 regression (bug 967220)...
> Seems like a perfect back-out candidate.

But it didn't cause this one... I don't see what bug 962677 has to do with this bug at all.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

(In reply to Dão Gottwald [:dao] from comment #8)
> Bug 962677 is marked P-. It blocks a P2 meta bug, but surely not every bug
> in that dependency list can count as a P2 on its own.

No, but the effort to smooth the transition is a P2, and bug 962677 was deemed an important progression in that effort, so it might as well be likened to a P2.

Comment 12

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #10)
> (In reply to Dão Gottwald [:dao] from comment #9)
> > And incidentally, bug 962677 caused another P2 regression (bug 967220)...
> > Seems like a perfect back-out candidate.
> 
> But it didn't cause this one... I don't see what bug 962677 has to do with
> this bug at all.

It apparently made Mike skip the calculations we did for the tabs toolbar on OS X. See bug 930094 comment 79.

Comment 13

[Justin Dolske [:Dolske]]

Is http://screencast.com/t/xEcwz6qsz an example of this bug?

Comment 14

[:Gijs (he/him)]

(In reply to Justin Dolske [:Dolske] from comment #13)
> Is http://screencast.com/t/xEcwz6qsz an example of this bug?

I think the background looks worse than this now.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

(In reply to Justin Dolske [:Dolske] from comment #13)
> Is http://screencast.com/t/xEcwz6qsz an example of this bug?

Yeah, Gijs is right - this is when it "worked". Now we're slightly more busted, in that the titlebar background doesn't extend down dynamically to fill the empty space.

But our old solution wasn't very pretty either.

Comment 16

[Justin Dolske [:Dolske]]

P4 based on this not seeming to be a real-world issue. At least yet!

Comment 17

[Dão Gottwald [:dao]]

Attachment: Bug 967917 - Enable browser_tabbar_big_widgets.js across platforms. r?mconley

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3cb436aa13e
Enable browser_tabbar_big_widgets.js across platforms. r=mconley

Comment 19

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f3cb436aa13e