Bug 67614

Summary: <meter> doesn't update rendering when its value is changed
Product: WebKit Reporter: Ian 'Hixie' Hickson <ian>
Component: Layout and RenderingAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, dominicc, hyatt, morrita, simon.fraser, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.hixie.ch/tests/adhoc/html/forms/meter/001.html
Attachments:
Description Flags
Patch
none
Patch for landing morrita: commit-queue-

Description Ian 'Hixie' Hickson 2011-09-05 12:09:34 PDT
Click the button in the testcase:
   http://www.hixie.ch/tests/adhoc/html/forms/meter/001.html

It should update the rendering. It doesn't. For some reason, clicking the meter itself does cause it to repaint with the new value.
Comment 1 Dominic Cooney 2011-09-05 13:46:01 PDT
Debugging this is tricky because shifting focus from Safari to the debugger will cause this to repaint. Modify the repro to update the value in a timeout and switch focus away from Safari before the timeout fires.

The relevant part of the tree looks like this:

BODY    0x1085106c0
        …
        P       0x108510e90
                #text   0x108510fe0 "\n "
                METER   0x1085111f0
                        #shadow-root    0x1085113a0
                                DIV     0x1085112a0
*                                       DIV     0x108511320 STYLE=width: 30%;

Style recalc gets down to the DIV, but it opts to not update its style in Element::recalcStyle, despite needing recalc, because hasParentStyle is false. But I am not sure how inline style recalc normally works :)
Comment 2 Dominic Cooney 2011-09-05 13:58:38 PDT
MeterShadowElement probably claims it doesn’t need a renderer in rendererIsNeeded, and this means the parent DIV of the value doesn’t have a renderer and hence doesn’t have a style. I note that adding <style>meter { -webkit-appearance: none; }</style> makes the bar update as expected.

Is the right thing here to use the Node::nonRendererRenderStyle hook that HTMLOptionElement uses?
Comment 3 Hajime Morrita 2011-09-20 03:38:35 PDT
Created attachment 107981 [details]
Patch
Comment 4 Hajime Morrita 2011-09-20 03:39:13 PDT
Thanks for Dominic's investigation, the fix itself is trivial.
Comment 5 Dimitri Glazkov (Google) 2011-09-20 09:10:49 PDT
Comment on attachment 107981 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=107981&action=review

> Source/WebCore/html/HTMLMeterElement.cpp:232
> +    if (RenderObject* render = renderer())
> +        render->updateFromElement();

Aren't we doubling the rate of repaints for shadow DOM-based meter?
Comment 6 Hajime Morrita 2011-09-20 22:28:04 PDT
(In reply to comment #5)
> (From update of attachment 107981 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=107981&action=review
> 
> > Source/WebCore/html/HTMLMeterElement.cpp:232
> > +    if (RenderObject* render = renderer())
> > +        render->updateFromElement();
> 
> Aren't we doubling the rate of repaints for shadow DOM-based meter?
I think the impact is little because actual repaints are deferred and batched into
single paint() call even though repaint() call will increase.
Comment 7 Hajime Morrita 2011-09-20 22:34:33 PDT
Created attachment 108109 [details]
Patch for landing
Comment 8 Hajime Morrita 2011-09-20 22:35:32 PDT
Comment on attachment 108109 [details]
Patch for landing

I should land this manually since bot cannot see this.
Comment 9 Hajime Morrita 2011-09-20 22:40:49 PDT
Committed r95599: <http://trac.webkit.org/changeset/95599>