[webkit-reviews] review denied: [Bug 82989] Auto-size may not work on first load : [Attachment 135246] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 2 18:12:59 PDT 2012


David Levin <levin at chromium.org> has denied Andrei Burago
<aburago at chromium.org>'s request for review:
Bug 82989: Auto-size may not work on first load
https://bugs.webkit.org/show_bug.cgi?id=82989

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

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=135246&action=review


>> Source/WebCore/ChangeLog:8
>> +	    No new tests. (OOPS!)
> 
> You should remove the 'No new tests' and either add and list tests, or
explain why no new tests were possible.  [changelog/nonewtests] [5]

What style queue said.

> Source/WebCore/dom/Document.cpp:2382
> +    m_processingLoadEvent = false;

There are two places where "return" is done above without setting this to false
above.

I wonder if we should use TemporaryChange here (although that can be tricky
because where you want to do the reset isn't the end of a scope, so you either
need to create a new scope, add a method to reset it early or allocate a
TemporaryChange into an OwnPtr and then clear it here).


More information about the webkit-reviews mailing list