Bug 118500 – [WK2][CoordinatedGraphics] Conform transforming functions to their names.

• Status: NEW
• Component: WebKit EFL

[Eunmi Lee]

The transforming functions in the WebView do not transform correctly.

The transformToScene() function scales the content position with content scale factor even though it is already scaled value.
The userViewportToContents() function transforms scene position instead of user-viewport position to the contents position.
The contentsToUserViewport() function transforms contents position to scene instead of user-viewport.

The scene position indicates the position in the window (Evas in the EFL), and the user-viewport position indicates the position in the web view (Evas_Object in the EFL).
The scene and user-viewport are different, so they should not be confused.

[Eunmi Lee]

Created attachment 206309 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 206309 [details]
Patch

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

These things are tricky, please get someone form the Nix port to have an extra look.

> Source/WebKit2/ChangeLog:7
> +        Reviewed by NOBODY (OOPS!).
> +

why no tests? Can you see if it is possible to add?

[Build Bot]

Comment on attachment 206309 [details]
Patch

Attachment 206309 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/973780

New failing tests:
fullscreen/full-screen-iframe-with-max-width-height.html

[Build Bot]

Created attachment 206362 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Mikhail Pozdnyakov]

Comment on attachment 206309 [details]
Patch

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

>> Source/WebKit2/ChangeLog:7
>> +
> 
> why no tests? Can you see if it is possible to add?

Agree with Kenneth, there should be a test showing the current problem in browser behaviour (it exists, right? since you're fixing it) and that this problem is solved after the patch is applied.

[Eunmi Lee]

Comment on attachment 206309 [details]
Patch

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

>>> Source/WebKit2/ChangeLog:7
>>> +
>> 
>> why no tests? Can you see if it is possible to add?
> 
> Agree with Kenneth, there should be a test showing the current problem in browser behaviour (it exists, right? since you're fixing it) and that this problem is solved after the patch is applied.

I'm sorry but I can not make test for this problem now, because I've found this problem while making gestures.
The gestures does not work correctly without this patch especially, scaling gesture using double tap.

I thought it is better that this patch is merged before gesture patch, so I made this patch.
Do I have to merge this patch after gesture patches?

[Eunmi Lee]

I've made new bug for correcting wrong usage of m_contentPosition in the WebView - Bug 118548,
So, I will update patch for this bug.

[Eunmi Lee]

Created attachment 206427 [details]
Patch

[Eunmi Lee]

Created attachment 206428 [details]
Patch

Rebased.

[Gyuyoung Kim]

LGTM. Noam ?

[Eunmi Lee]

Created attachment 221719 [details]
Patch

Rebase, add new API and update changelog.

[Jinwoo Song]

Comment on attachment 221719 [details]
Patch

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

> Source/WebKit2/ChangeLog:10
> +        anapplication. In the EFL, user viewport is the ewk_view (Evas_Object)

typo: anapplication.
In EFL port, the user viewport corresponds to Evas_Object(ewk_view) and the scene corresponds to Evas.

[Eunmi Lee]

Comment on attachment 221719 [details]
Patch

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

>> Source/WebKit2/ChangeLog:10
>> +        anapplication. In the EFL, user viewport is the ewk_view (Evas_Object)
> 
> typo: anapplication.
> In EFL port, the user viewport corresponds to Evas_Object(ewk_view) and the scene corresponds to Evas.

oops, anapplication -> an application.
Thanks, I will apply that.

[Eunmi Lee]

Created attachment 221724 [details]
Patch

Update changelog for Jinwoo's comments.

[Michael Catanzaro]

Comment on attachment 221724 [details]
Patch

Hi,

Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting.

To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.