Closed Bug 624101 Opened 9 years ago Closed 9 years ago

Make our canvases be moz-opaque

Categories

(Firefox Graveyard :: Panorama, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 4.0b10

People

(Reporter: mitcho, Assigned: ttaubert)

References

Details

(Keywords: perf, Whiteboard: [qa-][good first bug])

Attachments

(1 file, 3 obsolete files)

According to this page [1] canvas in Mozilla can have the dom attribute "dom-opaque"="true" (bug 430906) and this could speed up their rendering.

[1] https://developer.mozilla.org/En/Firefox_3.5_for_developers

Since, I believe all our tab canvases are always opaque, we should be slapping this on all our canvases.
(In reply to comment #0)
> According to this page [1] canvas in Mozilla can have the dom attribute
> "dom-opaque"="true" (bug 430906) and this could speed up their rendering.

Whoops! Make that "moz-opaque".
Assignee: nobody → tim.taubert
Status: NEW → ASSIGNED
Attached patch patch v1 (obsolete) — Splinter Review
Do we need a test for this?
Attachment #503289 - Flags: review?(ian)
Comment on attachment 503289 [details] [diff] [review]
patch v1

Looks good to me, bug I'd also like to hear from roc, for verification that this is a good idea.

No test needed.
Attachment #503289 - Flags: review?(ian)
Attachment #503289 - Flags: review+
Attachment #503289 - Flags: feedback?(roc)
Attachment #503289 - Flags: approval2.0?
Duplicate of this bug: 626284
Someone approve this! It's an easy win.
Attachment #503289 - Flags: approval2.0? → approval2.0+
Attached patch patch for checkin (obsolete) — Splinter Review
Attachment #503289 - Attachment is obsolete: true
Whiteboard: [qa-][good first bug][perf] → [qa-][good first bug]
http://hg.mozilla.org/mozilla-central/rev/653ad2c5782e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
Blocks: 604213
Should've been moz-opaque='', though.
Depends on: 626945
(In reply to comment #8)
> Should've been moz-opaque='', though.

Why so? 'true' seems to be working.
(In reply to comment #9)
> (In reply to comment #8)
> > Should've been moz-opaque='', though.
> 
> Why so? 'true' seems to be working.

Right, and when someone wants to revert this, they'll use moz-opaque='false' and have a hard time finding out why that doesn't change anything. See <http://www.whatwg.org/html/#boolean-attributes>.
Agree with Ms2ger on ='true' being bug-bait. But shorter's better, so why not use just moz-opaque (no ='' or =anything)?

/be
Attached patch patch v2, followup (obsolete) — Splinter Review
Attachment #504346 - Attachment is obsolete: true
Attachment #505399 - Flags: review?(ian)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Version: unspecified → Trunk
Comment on attachment 505399 [details] [diff] [review]
patch v2, followup

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Should've been moz-opaque='', though.
> > 
> > Why so? 'true' seems to be working.
> 
> Right, and when someone wants to revert this, they'll use moz-opaque='false'
> and have a hard time finding out why that doesn't change anything. See
> <http://www.whatwg.org/html/#boolean-attributes>.

I see! Thanks for setting us straight!
Attachment #505399 - Flags: review?(ian) → review+
Attachment #505399 - Flags: approval2.0?
Attachment #505399 - Flags: approval2.0? → approval2.0+
Attachment #505399 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/32738fd1cdfb
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.