Closed Bug 871279 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Theme, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: ge3k0s, Assigned: Gijs)

References

()

Details

(Whiteboard: [Australis:M6])

Attachments

(2 files)

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.
Attached image Titlebar bug
Summary: (Australis) Some pop-up tabs show the tabs too high in the titlebar → (Australis) Some pop-up windows show the tabs too high in the titlebar
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [Australis:M?]
Whiteboard: [Australis:M?] → [Australis:M6]
Assignee: nobody → gijskruitbosch+bugs
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?
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?
Flags: needinfo?(mconley)
(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.
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!
(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? :-)
Attached patch PatchSplinter Review
I think this is what we want?
Attachment #758181 - Flags: review?(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.
Attachment #758181 - Flags: review?(mconley) → review+
Whiteboard: [Australis:M6] → [Australis:M6][fixed-in-ux]
https://hg.mozilla.org/mozilla-central/rev/8527c9d9e340
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M6][fixed-in-ux] → [Australis:M6]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.