Bug 39479

Summary: Add DeviceOrientation and DeviceOrientationClient
Product: WebKit Reporter: Steve Block <steveblock>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, gustavo, hans, jorlow, steveblock, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 30335    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Steve Block
Reported 2010-05-21 03:24:22 PDT
Add DeviceOrientation and DeviceOrientationClient
Attachments
Patch (18.75 KB, patch)
2010-05-21 03:44 PDT, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (30.29 KB, patch)
2010-05-21 06:53 PDT, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (30.86 KB, patch)
2010-05-21 07:50 PDT, Steve Block 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Steve Block
Comment 1 2010-05-21 03:27:18 PDT
This is the next step in implementing DeviceOrientation. See Bug 30335
Steve Block
Comment 2 2010-05-21 03:44:29 PDT
Created attachment 56692 [details] Patch
Eric Seidel (no email)
Comment 3 2010-05-21 03:53:43 PDT
Attachment 56692 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/2248475
Early Warning System Bot
Comment 4 2010-05-21 03:54:48 PDT
Attachment 56692 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/2288390
WebKit Review Bot
Comment 5 2010-05-21 04:03:22 PDT
Attachment 56692 [details] did not build on gtk: Build output: http://webkit-commit-queue.appspot.com/results/2264396
Jeremy Orlow
Comment 6 2010-05-21 04:03:34 PDT
Comment on attachment 56692 [details] Patch You forget WebCore/CMakeLists.txt and visual studio. WebCore/dom/DeviceOrientation.h:38 + // TODO: Add methods to start and stop the service. FIXME WebCore/dom/DeviceOrientation.cpp:45 + // TODO: Fire DeviceOrientationEvents on the window object of all frames FIXME
Steve Block
Comment 7 2010-05-21 04:16:55 PDT
> You forget WebCore/CMakeLists.txt and visual studio. Looks like I did with my first patch too - will fix here. > WebCore/dom/DeviceOrientation.h:38 > + // TODO: Add methods to start and stop the service. > FIXME Done
WebKit Review Bot
Comment 8 2010-05-21 04:19:29 PDT
Attachment 56692 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2261407
Steve Block
Comment 9 2010-05-21 06:53:27 PDT
Created attachment 56705 [details] Patch
Jeremy Orlow
Comment 10 2010-05-21 07:00:56 PDT
Comment on attachment 56705 [details] Patch WebCore/ChangeLog:14 + No new tests are possible at this time. Please explain why. And promise to land them soon. :-) WebCore/dom/DeviceOrientation.cpp:47 + UNUSED_PARAM(alpha); You may need to include wtf/UnusedParam.h WebKit/chromium/src/WebViewImpl.cpp:264 + 0)); I don't know why these are split across multiple lines like this, but it doesn't match WebKit style...so mind putting these all on one line? WebKit/mac/WebView/WebView.mm:666 + _private->page = new Page(new WebChromeClient(self), new WebContextMenuClient(self), new WebEditorClient(self), new WebDragClient(self), new WebInspectorClient(self), new WebPluginHalterClient(self), geolocationControllerClient, deviceOrientationClient); Where is deviceOrientationClient defined?
Steve Block
Comment 11 2010-05-21 07:09:18 PDT
(In reply to comment #10) > (From update of attachment 56705 [details]) > WebCore/ChangeLog:14 > + No new tests are possible at this time. > Please explain why. And promise to land them soon. :-) Done > WebCore/dom/DeviceOrientation.cpp:47 > + UNUSED_PARAM(alpha); > You may need to include wtf/UnusedParam.h Done > WebKit/chromium/src/WebViewImpl.cpp:264 > + 0)); > I don't know why these are split across multiple lines like this, but it doesn't match WebKit style...so mind putting these all on one line? Done > WebKit/mac/WebView/WebView.mm:666 > + _private->page = new Page(new WebChromeClient(self), new WebContextMenuClient(self), new WebEditorClient(self), new WebDragClient(self), new WebInspectorClient(self), new WebPluginHalterClient(self), geolocationControllerClient, deviceOrientationClient); > Where is deviceOrientationClient defined? On the line above. Since all the other params are named, I thought it best to do it like this rather than use 0. What do you think?
Jeremy Orlow
Comment 12 2010-05-21 07:12:35 PDT
> > WebKit/mac/WebView/WebView.mm:666 > > + _private->page = new Page(new WebChromeClient(self), new WebContextMenuClient(self), new WebEditorClient(self), new WebDragClient(self), new WebInspectorClient(self), new WebPluginHalterClient(self), geolocationControllerClient, deviceOrientationClient); > > Where is deviceOrientationClient defined? > On the line above. Since all the other params are named, I thought it best to do it like this rather than use 0. What do you think? Oh yeah...forgot objective c always names stuff like that.
Steve Block
Comment 13 2010-05-21 07:50:27 PDT
Created attachment 56708 [details] Patch
Jeremy Orlow
Comment 14 2010-05-21 07:56:41 PDT
Comment on attachment 56708 [details] Patch r=me
Steve Block
Comment 15 2010-05-21 09:07:42 PDT
Comment on attachment 56708 [details] Patch Clearing flags on attachment: 56708 Committed r59935: <http://trac.webkit.org/changeset/59935>
Steve Block
Comment 16 2010-05-21 09:07:55 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.