863104 - Dock progress drawing glitch on Mac OS X 10.6

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Dave Vasilevsky [:vasi]]

Attachment: Screenshot of glitch

On Mac OS X 10.6, the download progress bar in the Firefox Dock icon is drawn strangely. It looks like the background and foreground of the progress bar aren't being drawn in the same location, even though they're both drawn by a single call to HIThemeDrawTrack().

Comment 1

[Dave Vasilevsky [:vasi]]

Attachment: Fix, but requires friend declaration in nsNativeThemeCocoa

This seems to have a cause similar to the issues in bug 465069, where Carbon drawing code wants to operate on a flipped graphics context.

Attached patch uses the existing code in nsNativeThemeCocoa to draw the progress bar, which already knows how to deal with this issue. The patch works fine on at least 10.7, 10.6 and 10.4 (TenFourFox). It does, however, require instantiating nsNativeThemeCocoa in nsMacDockSupport, and requires letting nsMacDockSupport use the protected DrawProgress() method (with a friend declaration).

Is that an OK way to expose this functionality to nsMacDockSupport? Is there a better way to do it?

Comment 2

[Markus Stange [:mstange]]

I think just making DrawProgress public would be fine.

Comment 3

[Josh Aas]

Comment on attachment 738817 [details] [diff] [review]
Fix, but requires friend declaration in nsNativeThemeCocoa

I agree with Markus, just make DrawProgress public.

Comment 4

[Dave Vasilevsky [:vasi]]

Attachment: Fix, using public DrawProgress()

Does this need review from anyone else, Dão maybe? UI review?

Comment 5

[Josh Aas]

Comment on attachment 740038 [details] [diff] [review]
Fix, using public DrawProgress()

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

IMO this needs no further review. Thanks for the fix!

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/63a501b7c61d

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/63a501b7c61d

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Can we get an uplift nom to aurora?

Comment 9

[Dave Vasilevsky [:vasi]]

Comment on attachment 740038 [details] [diff] [review]
Fix, using public DrawProgress()

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 548763
User impact if declined: Users on 10.6 will see broken progress bar during downloads
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): Little risk, the only change is to the drawing code for the dock progress bar. The only real alternative is delaying the entire feature to Fx 23.

Comment 10

[Dave Vasilevsky [:vasi]]

checkin-needed for mozilla-aurora

Comment 11

[:Felipe Gomes (needinfo for replies!)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/740aa9fbe16c

Comment 12

[Paul Silaghi, QA [:pauly]]

Reproduced the issue on nightly 2013-04-17.
Verified fixed FF 23b4 Mac OS X 10.6.8