Closed Bug 734003 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox13 affected, firefox14 verified)

VERIFIED FIXED
Firefox 14
Tracking Status
firefox13 --- affected
firefox14 --- verified

People

(Reporter: kats, Assigned: wlach)

Details

Attachments

(1 file)

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.
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)
Assignee: nobody → wlachance
Attachment #606741 - Flags: review?(pwalton)
I didn't write that XXX comment.
(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 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.
Attachment #606741 - Flags: review?(pwalton) → review+
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...
Attachment #606741 - Flags: feedback?(bgirard)
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.
Attachment #606741 - Flags: feedback?(bgirard) → feedback+
(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.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/14c95d1e5f35
Keywords: checkin-needed
Whiteboard: maple
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/14c95d1e5f35
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.