Closed Bug 786913 Opened 12 years ago Closed 12 years ago

[Azure] 0px fonts don't short circuit on text measuring and drawing

Categories

(Core :: Graphics: Canvas2D, defect)

Product:
Core
Component:
Graphics: Canvas2D
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18

People

(Reporter: ajones, Assigned: ajones)

References

Details

Attachments

(1 file, 3 obsolete files)

Reftest and fix for 0px font short circuit code v4
3.42 KB, patch
Details | Diff | Splinter Review 
Assertion when drawing shadowed 0 point fonts. Short circuit code needs to be added for 0 point fonts.
Comment on attachment 656736 [details] [diff] [review]
Reftest and fix for 0px font short circuit code

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

::: content/canvas/crashtests/0px-size-font-shadow.html
@@ +5,5 @@
> +  var canv = document.getElementById("canv");
> +  var ctx = canv.getContext("2d");
> +
> +  ctx.fillStyle = "red";
> +  // 0 size font shouldn't crash!

"shouldn't assert"

@@ +14,5 @@
> +  document.documentElement.className = "";
> +}
> +
> +</script>
> +<body onload="draw()">

This doesn't need to be onload. It's simpler to just put the <script> element after the canvas and have the script draw into the canvas directly. You don't even need a function definition.

::: content/canvas/src/nsCanvasRenderingContext2DAzure.cpp
@@ +3217,5 @@
> +  gfxFontGroup* currentFontStyle = GetCurrentFontStyle();
> +  NS_ASSERTION(currentFontStyle, "font group is null");
> +
> +  if (currentFontStyle->GetStyle()->size == 0.0F) {
> +    aWidth = 0;

this line is wrong :-)
Attachment #656736 - Flags: review?(roc) → review-
Try run for d205af5f951a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d205af5f951a
Results (out of 16 total builds):
    exception: 9
    failure: 7
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-d205af5f951a
Let's try that again
Attachment #656736 - Attachment is obsolete: true
Attachment #656749 - Flags: review?(roc)
Comment on attachment 656749 [details] [diff] [review]
Reftest and fix for 0px font short circuit code v2

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

r+ with that fixed

::: content/canvas/crashtests/0px-size-font-shadow.html
@@ +1,1 @@
> +<html xmlns="http://www.w3.org/1999/xhtml" >

You don't need this xmlns declaration, it's meaningless. You should have a <!DOCTYPE HTML> to ensure you're in standards mode.

@@ +1,2 @@
> +<html xmlns="http://www.w3.org/1999/xhtml" >
> +<body onload="draw()">

remove onload attribute
Attachment #656749 - Flags: review?(roc) → review+
Comment on attachment 656752 [details] [diff] [review]
Reftest and fix for 0px font short circuit code v3

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

::: content/canvas/crashtests/0px-size-font-shadow.html
@@ +1,4 @@
> +<!DOCTYPE HTML>
> +<body>
> +<canvas id="canv" width="5" height="5"></canvas>
> +<script language=javascript>

you can get rid of "language=javascript" too
Try run for 6f05b7ea024f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6f05b7ea024f
Results (out of 42 total builds):
    exception: 11
    success: 29
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ajones@mozilla.com-6f05b7ea024f
https://hg.mozilla.org/mozilla-central/rev/f9922b42205a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: