Bug 68491

Summary: Get rid of Widget::setBoundsSize
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, fsamuel, gustavo.noronha, gustavo, simon.fraser, thakis, vsevik, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Chromium failure test case. none

Description Anders Carlsson 2011-09-20 17:35:43 PDT
Get rid of Widget::setBoundsSize
Comment 1 Anders Carlsson 2011-09-20 17:41:06 PDT
Created attachment 108087 [details]
Patch
Comment 2 Early Warning System Bot 2011-09-20 17:50:46 PDT
Comment on attachment 108087 [details]
Patch

Attachment 108087 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9772473
Comment 3 Gyuyoung Kim 2011-09-20 17:52:02 PDT
Comment on attachment 108087 [details]
Patch

Attachment 108087 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9772475
Comment 4 Anders Carlsson 2011-09-20 17:52:08 PDT
Created attachment 108090 [details]
Patch
Comment 5 mitz 2011-09-20 17:57:18 PDT
Comment on attachment 108090 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        This basically reverts http://trac.webkit.org/changeset/79167 and http://trac.webkit.org/changeset/75897
> +        which were added to make zoom gestures work with WebKit1, but since we don't support zoom gestures in WebKit1
> +        anymore, it's better for code cleanliness to get rid of Widget::setBoundsSize and the associated code.

<http://trac.webkit.org/r75897> included a fix for bug 15676. Transformed iframes are supported in WebKit1 and this change looks like it will break them.
Comment 6 WebKit Review Bot 2011-09-20 18:41:09 PDT
Comment on attachment 108090 [details]
Patch

Attachment 108090 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9765547
Comment 7 WebKit Review Bot 2011-09-20 19:19:47 PDT
Comment on attachment 108090 [details]
Patch

Attachment 108090 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9772511
Comment 8 Collabora GTK+ EWS bot 2011-09-20 21:04:16 PDT
Comment on attachment 108090 [details]
Patch

Attachment 108090 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9773528
Comment 9 Anders Carlsson 2011-09-21 10:33:04 PDT
Created attachment 108176 [details]
Patch
Comment 10 mitz 2011-09-21 10:49:54 PDT
Comment on attachment 108176 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        This basically reverts http://trac.webkit.org/changeset/79167 and http://trac.webkit.org/changeset/75897
> +        which were added to make zoom gestures work with WebKit1, but since we don't support zoom gestures in WebKit1
> +        anymore, it's better for code cleanliness to get rid of Widget::setBoundsSize and the associated code.

Safari zoom gestures are one way to apply transforms to iframes and plug-ins. Another way is CSS transforms specified in the content.
Comment 11 Anders Carlsson 2011-09-21 10:54:07 PDT
(In reply to comment #10)

> 
> Safari zoom gestures are one way to apply transforms to iframes and plug-ins. Another way is CSS transforms specified in the content.

I'll update the ChangeLog to mention that.
Comment 12 Anders Carlsson 2011-09-21 12:17:59 PDT
Created attachment 108204 [details]
Patch
Comment 13 mitz 2011-09-21 12:23:17 PDT
Comment on attachment 108204 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        This basically reverts http://trac.webkit.org/changeset/79167 and http://trac.webkit.org/changeset/75897
> +        which were added to make zoom gestures work with WebKit1, but since we don't support zoom gestures in WebKit1
> +        anymore, it's better for code cleanliness to get rid of Widget::setBoundsSize and the associated code. While this
> +        will unfortunately break transformed iframes in WebKit1, but Simon says that it's an acceptable tradeoff.

What are the benefits of this change?
Comment 14 Anders Carlsson 2011-09-21 12:44:57 PDT
(In reply to comment #13)
> (From update of attachment 108204 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108204&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        This basically reverts http://trac.webkit.org/changeset/79167 and http://trac.webkit.org/changeset/75897
> > +        which were added to make zoom gestures work with WebKit1, but since we don't support zoom gestures in WebKit1
> > +        anymore, it's better for code cleanliness to get rid of Widget::setBoundsSize and the associated code. While this
> > +        will unfortunately break transformed iframes in WebKit1, but Simon says that it's an acceptable tradeoff.
> 
> What are the benefits of this change?

It removes a lot of cross platform code that's in reality only used by a single port, and makes the Widget class hierarchy easier to understand/refactor.
Comment 15 Anders Carlsson 2011-09-22 10:08:22 PDT
Comment on attachment 108204 [details]
Patch

Clearing flags on attachment: 108204

Committed r95725: <http://trac.webkit.org/changeset/95725>
Comment 16 Anders Carlsson 2011-09-22 10:08:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Vsevolod Vlasov 2011-09-23 11:23:54 PDT
This change breaks inspector (tested in chrome). 
Open docked inspector, resize it and after releasing mouse button move mouse up, without hovering inspector.
If you increased inspector size, you'll see white stripe in the bottom, because document body is not resized with the window

Inspector body has following style:

    position:absolute;
    top:0;
    left:0;
    bottom:0;
    right:0;

Once you hover inspector body is resized properly.
Comment 18 Vsevolod Vlasov 2011-09-23 12:30:20 PDT
Created attachment 108509 [details]
Chromium failure test case.
Comment 19 Nico Weber 2011-09-23 13:51:38 PDT
Also breaks window resizing in chromium: http://crbug.com/97787