[webkit-reviews] review granted: [Bug 120685] Create render tree lazily : [Attachment 219701] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 19 17:04:29 PST 2013


Andreas Kling <akling at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 120685: Create render tree lazily
https://bugs.webkit.org/show_bug.cgi?id=120685

Attachment 219701: patch
https://bugs.webkit.org/attachment.cgi?id=219701&action=review

------- Additional Comments from Andreas Kling <akling at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=219701&action=review


r=me!

> Source/WebCore/ChangeLog:100
> +	       TextControlInnerTextElement always preserveres newline even if
it doesn't have style yet.

Typo, preserves.

> Source/WebCore/dom/Element.cpp:2938
> +    auto children = elementDescendants(*this);
> +    for (auto it = children.begin(), end = children.end(); it != end; ++it)
{

Should use range for syntax here.

> Source/WebCore/dom/Node.cpp:2302
> +    // FIXME: This should go away alonw with the whole vague 'attached'
concept. The conditions here produce
> +    // roughly the old behavior based on an explicit attached bit.

Typo, along.

> Source/WebCore/html/HTMLElement.cpp:666
>  bool HTMLElement::supportsFocus() const
>  {
> +    if (!document().view()->isInLayout())
> +	   document().updateStyleIfNeeded();

I guess this function can be called during layout?

> Source/WebCore/html/HTMLFormControlElement.cpp:171
>  static bool shouldAutofocus(HTMLFormControlElement* element)

Urgh, raw pointer. How tacky.

> Source/WebCore/loader/PlaceholderDocument.cpp:40
> -    setAttached(true);
> +//	 setAttached(true);

Busted!


More information about the webkit-reviews mailing list