Expose willStartLiveResize and willEndLiveResize in WebView

• Component: New Bugs
• Status: RESOLVED FIXED

[Sailesh Agrawal]

Expose willStartLiveResize and willEndLiveResize in WebView

[Sailesh Agrawal]

Created attachment 92890 [details]
Patch

[Sailesh Agrawal]

The Chromium side of this change:
http://codereview.chromium.org/6998002/

[James Robinson]

+cc Darin, he approves all WebKit API changes.  What is a 'live resize'?

[Sailesh Agrawal]

(In reply to comment #3)
> What is a 'live resize'?
On the Mac a view is in live resize mode when the user clicks on the resize box and starts resizing the window. I believe the resize mode ends once the user lets go of the resize box.

See viewWillStartLiveResize and viewDidEndLiveResize here:
http://developer.apple.com/library/mac/#documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html

[Nico Weber]

Comment on attachment 92890 [details]
Patch

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

lgtm (but I'm not a reviewer)

> Source/WebKit/chromium/public/WebView.h:136
> +    // Notification that the view has started or ended live resize.

Maybe add "This is Mac-speak for when the user manuall resizes the window by dragging its corner." or something like this.

[Darin Fisher (:fishd, Google)]

Comment on attachment 92890 [details]
Patch

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

> Source/WebKit/chromium/public/WebView.h:137
> +    virtual void willStartLiveResize() = 0;

maybe you want to more explicitly mention the "resizer" in this API?  didStartDraggingResizer?  didStopDraggingResizer?

we already have the term "resizer" in the API.  see WebWidgetClient::windowResizerRect().

i'm a little concerned about using a OSX specific term here for something that could be generic to more ports.

[Nico Weber]

Comment on attachment 92890 [details]
Patch

FWIW, both the thing in WebCore this forwards to (frameview) and the thing on the chrome side call this "live resizing". I'm not sure if giving it a different name in the webkit api layer adds clarity (but I don't feel strongly).

[Darin Fisher (:fishd, Google)]

(In reply to comment #7)
> (From update of attachment 92890 [details])
> FWIW, both the thing in WebCore this forwards to (frameview) and the thing on the chrome side call this "live resizing". I'm not sure if giving it a different name in the webkit api layer adds clarity (but I don't feel strongly).

fair enough...  can i please see how this will be used in chromium?  i don't get what this is for.

[Nico Weber]

See http://codereview.chromium.org/6998002/ (comment 2)

[Nico Weber]

The next OS X version might add a new style of overlay scrollbars that are only visible while
a) the window is being scrolled or
b) the window is being resized.
To make b) happen, the scrollbars need to know if the window is being resized; that's what this is for afaiu.

[Sailesh Agrawal]

Created attachment 92963 [details]
Patch

[Sailesh Agrawal]

Comment on attachment 92890 [details]
Patch

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

>> Source/WebKit/chromium/public/WebView.h:136
>> +    // Notification that the view has started or ended live resize.
> 
> Maybe add "This is Mac-speak for when the user manuall resizes the window by dragging its corner." or something like this.

Done.

>> Source/WebKit/chromium/public/WebView.h:137
>> +    virtual void willStartLiveResize() = 0;
> 
> maybe you want to more explicitly mention the "resizer" in this API?  didStartDraggingResizer?  didStopDraggingResizer?
> 
> we already have the term "resizer" in the API.  see WebWidgetClient::windowResizerRect().
> 
> i'm a little concerned about using a OSX specific term here for something that could be generic to more ports.

As Nico mentioned, liveResize matches the naming in WebCore.

[Darin Fisher (:fishd, Google)]

(In reply to comment #10)
> The next OS X version might add a new style of overlay scrollbars that are only visible while
> a) the window is being scrolled or
> b) the window is being resized.
> To make b) happen, the scrollbars need to know if the window is being resized; that's what this is for afaiu.

so, we currently have the resize() method on WebWidget.  you can't resize an individual WebFrame, only the viewport of the topmost one.  it seems this pair of enter/exit "resizing" methods should accompany the resize() method, right?

  willBeginResizing();
  resize(size1);
  resize(size2);
  ...
  didFinishResizing();

I just picked "clean room" names that seem to go well with the action named "resize", but willStartLiveResize and willEndLiveResize work too.

[Darin Fisher (:fishd, Google)]

Comment on attachment 92963 [details]
Patch

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

> Source/WebKit/chromium/public/WebView.h:139
> +    virtual void willStartLiveResize() = 0;

please move to WebWidget.  i think you can try to define these better instead
of referring to some OSX behavior.  this API should be well defined so other
ports can understand how to use it.  see my previous note.  i think this is
intended to bound a sequence of resize commands so the implementation can show
UI related to such an interaction.

[Sailesh Agrawal]

Created attachment 93519 [details]
Patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 93519 [details]
Patch

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

Sorry if it comes off as though I'm being nit-picky here.  I just think it is
important that the API makes sense.

> Source/WebKit/chromium/public/WebWidget.h:58
> +    // Called when the user begins to drag the resizer.

You are still leaving things mysterious for the reader to guess what you mean.
I think you mean that willStartLiveResize will be followed by a sequence of
calls to resize(), but you don't say that.  Am I interpreting this API correctly?
If so, please add a comment to clarify this.

Is it required that this API only be used when resizing using the "resizer"?  Why
is the effect particular to that way of resizing?  what if there are other ways
of allowing a user gesture to actuate a "live resize" sequence?

I'm guessing (so please clarify), but I think you just want to tell WebKit to
treat a sequence of resize() calls as related to a single user gesture.  Maybe
that does not need to be particular to the "resizer" UI?

[Sailesh Agrawal]

Created attachment 94492 [details]
Patch

[Sailesh Agrawal]

(In reply to comment #16)
> (From update of attachment 93519 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=93519&action=review
> 
> Sorry if it comes off as though I'm being nit-picky here.  I just think it is
> important that the API makes sense.
> 
> > Source/WebKit/chromium/public/WebWidget.h:58
> > +    // Called when the user begins to drag the resizer.
> 
> You are still leaving things mysterious for the reader to guess what you mean.
> I think you mean that willStartLiveResize will be followed by a sequence of
> calls to resize(), but you don't say that.  Am I interpreting this API correctly?
> If so, please add a comment to clarify this.

I guess the live resize begin/end messages are meant as hints and can be used to group any sequence of resize events.

I updated the change log to (hopefully) clarify this.

(By the way, thanks for your feedback. Sorry, I've been so slow on updating this patch.)

[Darin Fisher (:fishd, Google)]

Comment on attachment 94492 [details]
Patch

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

> Source/WebKit/chromium/public/WebWidget.h:61
> +    // lets go of the resizer.

nice, thanks!

[WebKit Commit Bot]

Comment on attachment 94492 [details]
Patch

Clearing flags on attachment: 94492

Committed r87333: <http://trac.webkit.org/changeset/87333>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Sailesh Agrawal]

This change was backed out, trying again.

[Sailesh Agrawal]

Created attachment 94894 [details]
Patch

[Sailesh Agrawal]

To avoid breaking the Chromium build I changed the live resize start/end function not to be pure virtual.

Once http://codereview.chromium.org/6998002/ has landed I'll change them to be pure virtual.

Please take another look, thanks!

[WebKit Commit Bot]

Comment on attachment 94894 [details]
Patch

Clearing flags on attachment: 94894

Committed r87417: <http://trac.webkit.org/changeset/87417>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.