871279 - (Australis) Some pop-up windows show the tabs too high in the titlebar

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[u428464]

On some websites when I open pop-ups windows the tabs are shown directly in the titlebar. It's a bit strange.

Example shown in the attachment : when I click on "Thucydide, Guerre du Péloponnèse, IV, 34, 3" on the first page the pop-up shows a weird UI.

Comment 1

[u428464]

Attachment: Titlebar bug

Comment 2

[:Gijs (he/him)]

This seems to be caused by setting toolbar=yes. I've updated the URL field with a jsbin link testcase (you can try removing toolbar=yes from there). Seems like the checks (re)introduced for bug 873449 aren't enough.

Mike and/or Dao, should we just also be checking the sizemode here?

Comment 3

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

I think toolbar=yes is partially responsible, and menubar=no is the other half of the story.

I think this can possible be solved with a re-tweak of your patch in bug 870849. Applying that patch, the bug still persists because the toolbar-menubar is display: none (so adding padding-bottom does nothing), but if we were to add padding-top to the TabsToolbar instead, that might give us what we need.

Something like:

#main-window[sizemode="normal"] #toolbar-menubar[autohide="true"][inactive="true"] ~ #TabsToolbar {
  margin-top: 15px;
}

Does that make sense?

Comment 4

[Matthew N. [:MattN]]

(In reply to Mike Conley (:mconley) from comment #3)
> but if we were to add padding-top to the TabsToolbar instead, that might
> give us what we need.
> 
> Something like:
> 
> #main-window[sizemode="normal"]
> #toolbar-menubar[autohide="true"][inactive="true"] ~ #TabsToolbar {
>   margin-top: 15px;
> }

Make sure this doesn't reintroduce the problem fixed in bug 813802 comment 108.

Also note that you said padding-top but your example uses margin-top.

Comment 5

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

Er, right - margin-top is what I ended up using, and it seemed to work out.

I just tested it, and it doesn't seem to reproduce the problem in bug 813802 comment 108 - but good memory!

Comment 6

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #3)
> I think toolbar=yes is partially responsible, and menubar=no is the other
> half of the story.
> 
> I think this can possible be solved with a re-tweak of your patch in bug
> 870849. Applying that patch, the bug still persists because the
> toolbar-menubar is display: none (so adding padding-bottom does nothing),
> but if we were to add padding-top to the TabsToolbar instead, that might
> give us what we need.
> 
> Something like:
> 
> #main-window[sizemode="normal"]
> #toolbar-menubar[autohide="true"][inactive="true"] ~ #TabsToolbar {
>   margin-top: 15px;
> }
> 
> Does that make sense?

Yes. I'd like to get through bug 870849 first, though, and then we can subtly adjust from there. Then we also have blame about why we're making each change etc., and can find possible regressions more easily. Does that make sense, too? :-)

Comment 7

[:Gijs (he/him)]

Attachment: Patch

I think this is what we want?

Comment 8

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

Comment on attachment 758181 [details] [diff] [review]
Patch

Yes, I think this is what we want.

However, I've noticed that if I open the window from the jsbin, right-click on the toolbar, and choose "Menu Bar", we get into the collapse-y state again.

I've filed bug 879506 to disable the "Menu Bar" toggle if we're in a window where the menubar is disabled. Current Release Firefox just ignores the toggle.

Comment 9

[:Gijs (he/him)]

Pushed, https://hg.mozilla.org/projects/ux/rev/8527c9d9e340

Comment 10

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/8527c9d9e340