Bug 118500 - [WK2][CoordinatedGraphics] Conform transforming functions to their names.
Summary: [WK2][CoordinatedGraphics] Conform transforming functions to their names.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eunmi Lee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-09 05:04 PDT by Eunmi Lee
Modified: 2016-09-17 07:20 PDT (History)
17 users (show)

See Also:


Attachments
Patch (5.84 KB, patch)
2013-07-09 05:20 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (554.14 KB, application/zip)
2013-07-09 20:25 PDT, Build Bot
no flags Details
Patch (5.63 KB, patch)
2013-07-10 23:12 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff
Patch (5.58 KB, patch)
2013-07-10 23:32 PDT, Eunmi Lee
no flags Details | Formatted Diff | Diff
Patch (8.54 KB, patch)
2014-01-20 20:59 PST, Eunmi Lee
no flags Details | Formatted Diff | Diff
Patch (8.56 KB, patch)
2014-01-20 23:41 PST, Eunmi Lee
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eunmi Lee 2013-07-09 05:04:09 PDT
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.
Comment 1 Eunmi Lee 2013-07-09 05:20:48 PDT
Created attachment 206309 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2013-07-09 14:19:03 PDT
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?
Comment 3 Build Bot 2013-07-09 20:25:46 PDT
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
Comment 4 Build Bot 2013-07-09 20:25:49 PDT
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
Comment 5 Mikhail Pozdnyakov 2013-07-09 21:03:31 PDT
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.
Comment 6 Eunmi Lee 2013-07-10 01:46:35 PDT
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?
Comment 7 Eunmi Lee 2013-07-10 21:43:02 PDT
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.
Comment 8 Eunmi Lee 2013-07-10 23:12:53 PDT
Created attachment 206427 [details]
Patch
Comment 9 Eunmi Lee 2013-07-10 23:32:56 PDT
Created attachment 206428 [details]
Patch

Rebased.
Comment 10 Gyuyoung Kim 2013-12-25 18:41:44 PST
LGTM. Noam ?
Comment 11 Eunmi Lee 2014-01-20 20:59:50 PST
Created attachment 221719 [details]
Patch

Rebase, add new API and update changelog.
Comment 12 Jinwoo Song 2014-01-20 23:37:16 PST
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.
Comment 13 Eunmi Lee 2014-01-20 23:39:22 PST
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.
Comment 14 Eunmi Lee 2014-01-20 23:41:49 PST
Created attachment 221724 [details]
Patch

Update changelog for Jinwoo's comments.
Comment 15 Michael Catanzaro 2016-09-17 07:20:04 PDT
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.