Bug 46538 - Canvas: Crash when setting a font with size in 'ex' units
Summary: Canvas: Crash when setting a font with size in 'ex' units
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Nobody
URL: http://samples.msdn.microsoft.com/iet...
Keywords:
Depends on: 46581
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-24 16:40 PDT by Andreas Kling
Modified: 2010-09-28 07:30 PDT (History)
12 users (show)

See Also:


Attachments
Proposed patch (4.17 KB, patch)
2010-09-26 08:23 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
Proposed patch v2 (1.59 KB, patch)
2010-09-27 05:04 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2010-09-24 16:40:41 PDT
This test on the IE9 test center reveals a bug in WebKit:
http://samples.msdn.microsoft.com/ietestcenter/html5/canvas/canvas-text-font-002.htm

The problem is in CSSPrimitiveValue::computeLengthDouble() which assumes the passed-in style has a usable font().
Comment 1 Alexey Proskuryakov 2010-09-25 16:13:25 PDT
Not a regression from Safari 5.0.2, but crashing on an ietestcenter test is not good.
Comment 2 mitz 2010-09-25 16:32:30 PDT
I think this can be fixed by calling newStyle->font().update() after setting the font description on the newStyle.
Comment 3 Andreas Kling 2010-09-26 08:23:12 PDT
Created attachment 68848 [details]
Proposed patch

Patch using mitz's suggestion.
Comment 4 Andreas Kling 2010-09-26 10:39:42 PDT
Comment on attachment 68848 [details]
Proposed patch

Clearing flags on attachment: 68848

Committed r68343: <http://trac.webkit.org/changeset/68343>
Comment 5 Andreas Kling 2010-09-26 10:39:52 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 WebKit Review Bot 2010-09-26 11:15:01 PDT
http://trac.webkit.org/changeset/68343 might have broken GTK Linux 32-bit Release
Comment 7 Andreas Kling 2010-09-26 12:31:03 PDT
Reopening since this broke GTK+.
Comment 8 Andreas Kling 2010-09-27 01:22:29 PDT
CC'ing GTK+ people for a look..
Comment 9 Andreas Kling 2010-09-27 05:04:57 PDT
Created attachment 68898 [details]
Proposed patch v2

Essentially the same patch, except do the update() even if !canvas()->computedStyle() (I guess if the element isn't attached to the document.)
Tested on Qt and GTK+.
Comment 10 WebKit Commit Bot 2010-09-27 11:13:24 PDT
Comment on attachment 68898 [details]
Proposed patch v2

Rejecting patch 68898 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 68898]" exit_code: 2
Cleaning working directory
Updating working directory
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/4033166
Comment 11 Eric Seidel (no email) 2010-09-27 11:32:21 PDT
Comment on attachment 68898 [details]
Proposed patch v2

False rejection.  Bug in the queue.  One of the cluster nodes seems stuck.
Comment 12 WebKit Commit Bot 2010-09-27 12:13:37 PDT
Comment on attachment 68898 [details]
Proposed patch v2

Rejecting patch 68898 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 68898]" exit_code: 2
Cleaning working directory
Updating working directory
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/4074125
Comment 13 Eric Seidel (no email) 2010-09-27 12:30:00 PDT
Comment on attachment 68898 [details]
Proposed patch v2

One of the cq nodes is misbehaving.  It should right itself shortly.  I can't access it to manually reboot it at this time.
Comment 14 WebKit Commit Bot 2010-09-27 13:14:50 PDT
Comment on attachment 68898 [details]
Proposed patch v2

Rejecting patch 68898 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 68898]" exit_code: 2
Cleaning working directory
Updating working directory
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2

Full output: http://queues.webkit.org/results/4066158
Comment 15 Eric Seidel (no email) 2010-09-27 13:15:57 PDT
Sorry.  Will wait until I know the CQ node is healthy again.
Comment 16 Eric Seidel (no email) 2010-09-27 13:26:13 PDT
Comment on attachment 68898 [details]
Proposed patch v2

I repaired one of them just now.  Git got very upset by some test rebaselinings this morning.  It was failing to update.
Comment 17 Eric Seidel (no email) 2010-09-27 18:51:58 PDT
Comment on attachment 68898 [details]
Proposed patch v2

Please leave the string "Reviewed by NOBODY (OOPS!)." in your ChangeLog if you want this to be handled by the commit-queue.
Comment 18 Andreas Kling 2010-09-28 07:25:51 PDT
Committed r68513: <http://trac.webkit.org/changeset/68513>
Comment 19 Andreas Kling 2010-09-28 07:30:33 PDT
Comment on attachment 68898 [details]
Proposed patch v2

> Please leave the string "Reviewed by NOBODY (OOPS!)." in your ChangeLog if you want this to be handled by the commit-queue.

D'oh! Thanks Eric.