[Webkit-unassigned] [Bug 73911] <li value="0"> behaves like <li> (the same for negative numbers)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73911





--- Comment #15 from Florin Malita <fmalita at google.com>  2011-12-07 12:11:23 PST ---
(In reply to comment #11)
> I don’t think the name of the function is good, though, because it doesn’t set the value on the element. Maybe setValueInRenderer or parseValue.

Good point. Using parseValue().


(In reply to comment #12)
> View in context: https://bugs.webkit.org/attachment.cgi?id=118228&action=review
> 
> > Source/WebCore/html/HTMLLIElement.cpp:-113
> > -        const AtomicString& requestedValueString = fastGetAttribute(valueAttr);
> > -        if (requestedValueString.isNull())
> > -            render->clearExplicitValue();
> 
> Are you saying that the isNull check is no longer needed, because valueOK will be false? Things like this should be explained in ChangeLog (this is why prepare-Script generates a list of modified functions).

ChangeLog updated.


> How will this affect performance? At a first glance, it feels like a bigger issue than "inline" on setValue that you added.

As far as the isNull() check goes, if the function gets inlined the optimized result should be equivalent (as Darin mentioned, toInt() performs pretty much the same check).

I've also extracted the renderer() && renderer()->isListItem() test back into parseMappedAttribute() to avoid evaluating it twice on the attach path.


> 
> > Source/WebCore/html/HTMLLIElement.cpp:111
> > +    if (renderer() && renderer()->isListItem()) {
> 
> WebKit style prefers early return.

Done (no longer present in parseValue()).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.