Bug 70901 - Name viewport change event consistently.
Summary: Name viewport change event consistently.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-26 03:38 PDT by zalan
Modified: 2015-08-07 13:42 PDT (History)
3 users (show)

See Also:


Attachments
Patch (29.20 KB, patch)
2011-10-26 04:14 PDT, zalan
kenneth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2011-10-26 03:38:55 PDT
Name viewport change event consistently.
Comment 1 zalan 2011-10-26 04:14:58 PDT
Created attachment 112479 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-26 04:16:32 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Kenneth Rohde Christiansen 2011-10-26 04:23:42 PDT
Comment on attachment 112479 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2194
> +void WebPageProxy::didChangeViewportProperties(const ViewportArguments& args)

Will you change this one as well?
Comment 4 Kenneth Rohde Christiansen 2011-10-26 04:24:39 PDT
Comment on attachment 112479 [details]
Patch

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

> Source/WebCore/page/Chrome.cpp:487
> -void Chrome::dispatchViewportDataDidChange(const ViewportArguments& arguments) const
> +void Chrome::dispatchViewportPropertiesDidChange(const ViewportArguments& arguments) const

Just to make sure, why isnt this called dispatchDidChangeViewportProperties? OR why the 

didChange* vs the *DidChange? Consistent with WebKit2?
Comment 5 zalan 2011-10-26 04:39:08 PDT
(In reply to comment #4)
> (From update of attachment 112479 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=112479&action=review
> 
> > Source/WebCore/page/Chrome.cpp:487
> > -void Chrome::dispatchViewportDataDidChange(const ViewportArguments& arguments) const
> > +void Chrome::dispatchViewportPropertiesDidChange(const ViewportArguments& arguments) const
> 
> Just to make sure, why isnt this called dispatchDidChangeViewportProperties? OR why the 
> 
> didChange* vs the *DidChange? Consistent with WebKit2?

WebKit2 has the style of didChangeSomething (DidChangeScrollbarsForMainFrame) while WebCore convention is more like somethingDidChange (scrollbarsModeDidChange).
Comment 6 Simon Hausmann 2011-10-26 06:39:46 PDT
Committed r98472: <http://trac.webkit.org/changeset/98472>