Bug 45081

Summary:

Crash rendering <meter/> with percent padding

Product:

WebKit

Reporter:

James Kozianski <koz>

Component:

Layout and Rendering

Assignee:

James Kozianski <koz>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, jamesr, morrita, noel.gordon, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Reproduction	none
Stack trace	none
Proposed patch	tkent: review-
Proposed patch	abarth: review-
Proposed patch	none

James Kozianski

Reported 2010-09-01 19:09:44 PDT

Created attachment 66312 [details] Reproduction WebKit crashes when trying to render a <meter> element that has percentage padding, eg: <meter style="padding: 1%;" /> Test case attached.

Attachments
Reproduction (31 bytes, text/html) 2010-09-01 19:09 PDT, James Kozianski	no flags	Details
Stack trace (1.56 KB, text/plain) 2010-09-01 20:46 PDT, James Kozianski	no flags	Details
Proposed patch (3.25 KB, patch) 2010-09-02 01:22 PDT, James Kozianski	tkent: review-	Details Formatted Diff Diff
Proposed patch (3.26 KB, patch) 2010-09-02 01:51 PDT, James Kozianski	abarth: review-	Details Formatted Diff Diff
Proposed patch (4.08 KB, patch) 2010-09-06 21:29 PDT, James Kozianski	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

James Kozianski

Comment 1 2010-09-01 20:46:18 PDT

Created attachment 66318 [details] Stack trace

James Kozianski

Comment 2 2010-09-01 21:22:24 PDT

The crash is caused by a null dereference of containingBlock() in RenderBoxModelObject::paddingTop(). RenderMeter queries its size when determining whether it needs to be layed out, but initially it has no containing block, hence the null dereference. I'll write a patch to make RenderMeter always request layout so this query never occurs.

James Kozianski

Comment 3 2010-09-02 01:22:10 PDT

Created attachment 66338 [details] Proposed patch

WebKit Review Bot

Comment 4 2010-09-02 01:24:17 PDT

Attachment 66338 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Hajime Morrita

Comment 5 2010-09-02 01:47:35 PDT

Comment on attachment 66338 [details] Proposed patch Hi, thank you for doing this! The change looks OK in general. Please fix what the bot claims. You can use WebKitTools/Scripts/check-webkit-style to correct coding convention errors before the bot bites us. For the test, are we OK even for other than padding? Having size-based properties like margin, width, height would be helpful. For ChangeLog, please mention what caused the crash briefly.

James Kozianski

Comment 6 2010-09-02 01:51:35 PDT

Created attachment 66339 [details] Proposed patch

Kent Tamura

Comment 7 2010-09-02 01:52:03 PDT

Comment on attachment 66338 [details] Proposed patch r- for the style error.

Kent Tamura

Comment 8 2010-09-05 23:32:45 PDT

The second patch seems not to answer Morita-san's requests. > For the test, are we OK even for other than padding? > Having size-based properties like margin, width, height would be helpful. > > For ChangeLog, please mention what caused the crash briefly.

Adam Barth

Comment 9 2010-09-05 23:43:39 PDT

Comment on attachment 66339 [details] Proposed patch See comment above.

James Kozianski

Comment 10 2010-09-06 21:29:27 PDT

Created attachment 66684 [details] Proposed patch

Hajime Morrita

Comment 11 2010-09-07 00:49:30 PDT

Looks fine for me.

Kent Tamura

Comment 12 2010-09-07 00:50:21 PDT

Comment on attachment 66684 [details] Proposed patch OK.

WebKit Commit Bot

Comment 13 2010-09-07 01:12:45 PDT

Comment on attachment 66684 [details] Proposed patch Clearing flags on attachment: 66684 Committed r66864: <http://trac.webkit.org/changeset/66864>

WebKit Commit Bot

Comment 14 2010-09-07 01:12:49 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.