Bug 82989 – Auto-size may not work on first load

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andrei Burago]

Auto-size may not work on first load

[Andrei Burago]

Created attachment 135246 [details]
Patch

[WebKit Review Bot]

Attachment 135246 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrei Burago]

Repro steps: 

1) Run Chrome and Install https://chrome.google.com/webstore/detail/gighmmpiobklfepjocnamgkkbiglidom
2.) left-click on the AdblockForChrome toolbar icon
3.) the pop-up menu is not completely to see.

TESTS: Manually repeat the repro steps

While layout is called from Document::implicitClose(), the tree does not seem to be ready. As soon as layout is called again for a different reason (activation, etc.) the layout results are back to normal.

This patch makes sure that the autosize does not shrink the visible area because of this temporary change in layout results.

[David Levin]

Comment on attachment 135246 [details]
Patch

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).

[David Levin]

Thank you very much for working on this!

[Andrei Burago]

Created attachment 135370 [details]
Patch

[David Levin]

Comment on attachment 135370 [details]
Patch

Thanks again!

[WebKit Review Bot]

Comment on attachment 135370 [details]
Patch

Rejecting attachment 135370 [details] from commit-queue.

New failing tests:
fast/autoresize/turn-off-autoresize.html
fast/autoresize/basic.html
Full output: http://queues.webkit.org/results/12320323

[WebKit Review Bot]

Created attachment 135385 [details]
Archive of layout-test-results from ec2-cq-02

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[WebKit Review Bot]

Comment on attachment 135370 [details]
Patch

Attachment 135370 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12325241

New failing tests:
fast/autoresize/turn-off-autoresize.html
fast/autoresize/basic.html

[WebKit Review Bot]

Created attachment 135403 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Andrei Burago]

Created attachment 135684 [details]
Patch

[Andrei Burago]

Updated the patch. The previous version did not give the window a chance to shrink (if needed) when autosize was enabled on load.

[WebKit Review Bot]

Comment on attachment 135684 [details]
Patch

Clearing flags on attachment: 135684

Committed r113287: <http://trac.webkit.org/changeset/113287>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.