Summary: | [BlackBerry] Expose WebViewportArguments to WebPageClient | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Lattanzio <mlattanzio> | ||||||||||
Component: | WebKit BlackBerry | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | kpiascik, rakuco, tonikitoo, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Other | ||||||||||||
OS: | Other | ||||||||||||
Attachments: |
|
Description
Mike Lattanzio
2012-04-20 11:15:34 PDT
Created attachment 138129 [details]
Patch
Created attachment 138130 [details]
Patch
Created attachment 138144 [details]
Patch
you still didn't explain in your patch why you need currentViewportArguments and are not overriding m_viewportArguments, I think this is the important thing to note....also do you have any tests? Created attachment 138148 [details]
Patch
(In reply to comment #4) > you still didn't explain in your patch why you need currentViewportArguments and are not overriding m_viewportArguments, I think this is the important thing to note....also do you have any tests? The new patch has more thorough comments. API tests for the WebViewportArguments are in launcher. This patch doesn't affect anything by default so no new tests for webkit. (In reply to comment #6) > (In reply to comment #4) > > you still didn't explain in your patch why you need currentViewportArguments and are not overriding m_viewportArguments, I think this is the important thing to note....also do you have any tests? > > The new patch has more thorough comments. API tests for the WebViewportArguments are in launcher. This patch doesn't affect anything by default so no new tests for webkit. Looks good to me. Comment on attachment 138148 [details]
Patch
still think d should not be a pointer, but ok.
Comment on attachment 138148 [details] Patch Clearing flags on attachment: 138148 Committed r114779: <http://trac.webkit.org/changeset/114779> All reviewed patches have been landed. Closing bug. |