734003 - Maple: using white as the base checkerboard color doesn't work

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

This code comment:

     // XXX We don't handle setting the color to white (-1),
     //     there a bug somewhere but I'm not sure where,
     //     let's hardcode the color for now to black and come back and fix 

should be fixed.

Comment 1

[William Lachance (:wlach)]

Attachment: Fix checkerboarding color after merge with maple

It looks like browser.js just needed some slight updates here. The attached wfm.

(assigning to :pcwalton as he was the person who wrote the original code, feel free to reassign if more convenient)

Comment 2

[Patrick Walton (:pcwalton)]

I didn't write that XXX comment.

Comment 3

[William Lachance (:wlach)]

(In reply to Patrick Walton (:pcwalton) from comment #2)
> I didn't write that XXX comment.

Right, but you did write the original code to set the checkerboard color. ;)

Comment 4

[Patrick Walton (:pcwalton)]

Comment on attachment 606741 [details] [diff] [review]
Fix checkerboarding color after merge with maple

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

I can rubber stamp this, I guess. I'd check with whoever wrote that XXX comment though.

Comment 5

[William Lachance (:wlach)]

Comment on attachment 606741 [details] [diff] [review]
Fix checkerboarding color after merge with maple

git blame suggests it was Benoit Girard?

Benoit: Can you give your feedback on this? It looks like things were just temporarily broken in a refactoring...

Comment 6

[Benoit Girard (:BenWa)]

Comment on attachment 606741 [details] [diff] [review]
Fix checkerboarding color after merge with maple

As long we we're handling checkerboarding correctly with varius background colors including white I'm fine by this.

Comment 7

[William Lachance (:wlach)]

(In reply to Benoit Girard (:BenWa) from comment #6)
> Comment on attachment 606741 [details] [diff] [review]
> Fix checkerboarding color after merge with maple
> 
> As long we we're handling checkerboarding correctly with varius background
> colors including white I'm fine by this.

Yup, everything is working as it was before the merge. Sites with a light background (e.g. taskjs.org) get a white/gray checkerboard, sites with a dark background (e.g. daringfireball.net) get a gray/back checkerboard.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/14c95d1e5f35

Comment 9

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/14c95d1e5f35

Comment 10

[Adrian Tamas (:AdrianT)]

Default checkerboard color has been changed to white. According to Comment 7 this implementation has worked at some point. Closing the issue as verified although this is no longer valid