Closed Bug 678002 Opened 8 years ago Closed 8 years ago

[MAC] Titlebar is black when a persona (lightweight theme) is applied on 10.6

Categories

(Core :: Widget: Cocoa, defect)

All
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla8

People

(Reporter: MattN, Assigned: mstange)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

Build ID: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a1) Gecko/20110810 Firefox/8.0a1 
Built from: http://hg.mozilla.org/mozilla-central/rev/04dfb49d3a3d

Starting with today's nightly build (on Snow Leopard 10.6.8) the window titlebar is black instead of showing the persona image in it.  The titlebar is normal with the the default theme.

STR:
1) Go to getpersonas.com and 'wear' a persona

Expected result:
The persona image is shown on all of the standard toolbars and in the titlebar

Actual result:
The persona image shows on the toolbars but the titlebar background is black (with black text). See screenshot.

I suspect this is caused by fixes for OS X Lion (eg. bug 668195 or bug 667476).
> I suspect this is caused by fixes for OS X Lion (eg. bug 668195 or bug 667476).

You're probably right.
Blocks: 668195, 667476
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Huh, strange. I see this bug in the downloaded nightly but not in my local debug build.
(In reply to Markus Stange from comment #3)
> Huh, strange. I see this bug in the downloaded nightly but not in my local
> debug build.

I noticed that too, my SeaMonkey debug build doesn't exibit this (built with your inbound patches, before they where pushed to m-c) but the latest nightly does.
I have no idea what's going on here. I can reproduce the bug in an opt build, and adding a printf at this place makes it go away. I'd love to know why.

I've looked at the disassembly of TitlebarDrawCallback in gdb before and after the patch, and the call to printf is really the only difference. Three additional instructions:
+	lea    0x815a5d(%rip),%rdi
+	xor    %eax,%eax
+	callq  0x1023369c8 <dyld_stub_printf>

The bug also goes away if I removed the call to setCurrentContext, but that of course causes other problems. It looks like setCurrentContext somehow leaves the world in a broken state, and the call to printf fixes things again.
Attached patch workaround (obsolete) — Splinter Review
printf isn't the only magical antidote; filling the background with a color works, too, and has no bad side-effects. Should we land this as a temporary workaround? Finding the real bug here would be better, of course, but at the moment I have no idea where to look for it.
Attachment #552356 - Flags: review?(joshmoz)
No longer blocks: 667476
Since this bug is so fiddly, I wonder if it only occurs in 64-bit mode.

(Markus, the assembler code you quoted in comment #5 is, of course, 32-bit assembler.)
> (Markus, the assembler code you quoted in comment #5 is, of course,
> 32-bit assembler.)

Actually, now that I look more closely, it actually has both 64-bit
assembler (the references to %rip and %rdi) and 32-bit assembler (the
references to %eax).  How odd.

I can't say for sure this is a problem, though.
(In reply to Steven Michaud from comment #8)
> I can't say for sure this is a problem, though.

I don't think that's the problem. According to https://github.com/yasm/yasm/wiki/AMD64 %eax just maps to the 32 least significant bits of %rax.
It also leaves the 32 most significant bits of %rax unchanged/uninitialized -- but again that might not be a problem.
I still think it's worth checking to see if this bug happens in 32-bit mode.
Comment on attachment 552356 [details] [diff] [review]
workaround

Review of attachment 552356 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with this workaround for now if you want to do it.
Attachment #552356 - Flags: review?(joshmoz) → review+
Attached patch better patchSplinter Review
OK, this looks better!

Before bug 668195 we didn't call +[NSGraphicsContext setCurrentContext:] in drawsContentsIntoWindowFrame mode. This patch restores that situation, and magically the bug goes away. It also fixes bug 678184.
Only NSRectFill really looks at [NSGraphicsContext currentContext], so I'm limiting the setCurrentContext call to that case.
Attachment #552356 - Attachment is obsolete: true
Attachment #552422 - Flags: review?(joshmoz)
Blocks: 678184
Asa was able to reproduce this on his 10.7 too.
Attachment #552422 - Flags: review?(joshmoz) → review+
http://hg.mozilla.org/mozilla-central/rev/bcab3415d9ee
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Verified fixed on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a1) Gecko/20110815 Firefox/8.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.