The default bug view has changed. See this FAQ.

[Mac][personas] lightweight theming crunches window title bar

RESOLVED FIXED in seamonkey2.20

Status

SeaMonkey
Themes
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Karsten Düsterloh, Assigned: stefanh)

Tracking

({regression})

Trunk
seamonkey2.20
x86
Mac OS X
regression

SeaMonkey Tracking Flags

(seamonkey2.18 fixed, seamonkey2.19 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 739980 [details]
crunched window title

When using lightweight theming on Mac, the window title bar is crunched into the window content. See attached image for details.

Self built:
User agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:23.0) Gecko/20100101 Firefox/23.0 SeaMonkey/2.20a1
Build identifier: 20130416223447
Mac OS X 10.6.8
(Assignee)

Comment 1

4 years ago
I see this too... interesting... without digging it looks like the height calculation doesn't include the titlebar height.
(Assignee)

Comment 2

4 years ago
Doesn't happen in todays firefox nightly. Do you have a regression range?
(Reporter)

Comment 3

4 years ago
> Doesn't happen in todays firefox nightly.

*nod* FF and TB built from the same codebase aren't affected.

> Do you have a regression range?

No(t yet). I *think* my last build before (of 2013-01-14 :-() was okay, though.
(Assignee)

Comment 4

4 years ago
Hmm, this might be caused by bug 647216. I haven't tested, so I'm guessing here :-)

LightweightThemeConsumer.jsm has this:

110 #ifdef XP_MACOSX
111     if (active)
112       root.setAttribute("drawintitlebar", "true");
113     else
114       root.removeAttribute("drawintitlebar");
115 #endif

And xul.css used to have this:
%ifdef XP_MACOSX
:root[drawintitlebar="true"] {
  padding-top: 22px;
}
%endif

But the above code in xul.css was removed in bug 647216:
https://hg.mozilla.org/mozilla-central/diff/2f54529528a9/toolkit/content/xul.css
(Assignee)

Comment 5

4 years ago
If you do
+:root[drawintitlebar="true"] {
+  padding-top: 22px;
+}
+
in suite/themes/classic/mac/communicator.css the problem goes away. But that'll cause bug 806244, so we'll need a better approach here...
(Assignee)

Comment 6

4 years ago
(We probably need a titlebar vbox in our main windows)
(Assignee)

Updated

4 years ago
Assignee: nobody → stefanh
(Assignee)

Comment 7

4 years ago
Created attachment 740113 [details] [diff] [review]
Fix title bar

I hope I got all windows now :-)

Optionally, this should be handled in toolkit/...

This will also be needed in aurora. I wanted to remove the height of the #titlebar height in fullscreen mode with a persona applied (#main-window[drawintitlebar="true][sizemode="fullscreen"] > #titlebar), but there was a delay in the height adjustment, so I didn't thought it looked good enough.

(the sr? is a moa request).
Attachment #740113 - Flags: superreview?(mnyromyr)
Attachment #740113 - Flags: review?(mnyromyr)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Keywords: regression
(Assignee)

Updated

4 years ago
status-seamonkey2.18: --- → affected
status-seamonkey2.19: --- → affected
(Assignee)

Comment 8

4 years ago
(In reply to Stefan [:stefanh] from comment #7)

> This will also be needed in aurora.

And in beta too, since the m-c patch landed before February 19.

Comment 9

4 years ago
Does Addressbook need one too?
(Assignee)

Comment 10

4 years ago
(In reply to Philip Chee from comment #9)
> Does Addressbook need one too?

Yes, but it's in the patch?

Comment 11

4 years ago
Could it not just go into utilityOverlay.xul?
(Reporter)

Comment 12

4 years ago
Comment on attachment 740113 [details] [diff] [review]
Fix title bar

>+  <hbox id="titlebar"/>

Looking at the way FF/TB use this pattern, this should be a <vbox>. (Just so that if someone ports the CAN_DRAW_IN_TITLEBAR stuff, he doesn't need to worry where and why the hbox came from.)

(In reply to Stefan [:stefanh] from comment #7)
> Optionally, this should be handled in toolkit/...

Is this likely? ;-)

(In reply to Ian Neal from comment #11)
> Could it not just go into utilityOverlay.xul?

How?! (Or maybe I misunderstand your proposal?)
Attachment #740113 - Flags: superreview?(mnyromyr)
Attachment #740113 - Flags: superreview+
Attachment #740113 - Flags: review?(mnyromyr)
Attachment #740113 - Flags: review+
(Assignee)

Comment 13

4 years ago
> I hope I got all windows now :-)

Turns out that I forgot messenger.xul and downloadmanager.xul...
(Assignee)

Comment 14

4 years ago
Created attachment 741438 [details] [diff] [review]
Updated patch, r+moa=Mnyromyr

(In reply to Stefan [:stefanh] from comment #13)
> > I hope I got all windows now :-)
> 
> Turns out that I forgot messenger.xul and downloadmanager.xul...

Just putting the patch here... tree looks busted atm.
(Assignee)

Comment 15

4 years ago
Comment on attachment 741438 [details] [diff] [review]
Updated patch, r+moa=Mnyromyr

Got a memo from Karsten, he is (naturally) OK with the updated patch
Attachment #741438 - Attachment description: Updated patch → Updated patch, r+moa=Mnyromyr
(Assignee)

Updated

4 years ago
Attachment #740113 - Attachment is obsolete: true
(Assignee)

Comment 16

4 years ago
Comment on attachment 741438 [details] [diff] [review]
Updated patch, r+moa=Mnyromyr

Starting tomorrow, I'll be away until around May 5-6. The tree seems really busted atm, so I'll land this when I get back. But if someone wants to drive this in to the tree when I'm away, feel free to do so (I'm a bit unsure when the next uplift is)

[Approval Request Comment]
Regression caused by: Bug 647216
User impact if declined: Window title bar appears over toolbar in lightweight themes (on Mac)
Testing completed: nope, see above 
Risk to taking this patch: Low
String changes made by this patch: None
Attachment #741438 - Flags: approval-comm-beta?
Attachment #741438 - Flags: approval-comm-aurora?

Updated

4 years ago
Attachment #741438 - Flags: approval-comm-beta?
Attachment #741438 - Flags: approval-comm-beta+
Attachment #741438 - Flags: approval-comm-aurora?
Attachment #741438 - Flags: approval-comm-aurora+
https://hg.mozilla.org/releases/comm-beta/rev/af57b5466466
status-seamonkey2.18: affected → fixed
(Assignee)

Comment 18

4 years ago
http://hg.mozilla.org/comm-central/rev/be604ac62ced
http://hg.mozilla.org/releases/comm-aurora/rev/329ad226ac9a
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-seamonkey2.19: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.20
You need to log in before you can comment on or make changes to this bug.