Bug 144906 - REGRESSION(r176631): [EFL] Fullscreen feature doesn't work correctly on MiniBrowser
Summary: REGRESSION(r176631): [EFL] Fullscreen feature doesn't work correctly on MiniB...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: daegyu.lee@navercorp.com
URL:
Keywords:
Depends on: 139109
Blocks:
  Show dependency treegraph
 
Reported: 2015-05-12 00:44 PDT by Gyuyoung Kim
Modified: 2015-05-13 02:15 PDT (History)
2 users (show)

See Also:


Attachments
screenshot (60.94 KB, image/png)
2015-05-12 00:48 PDT, Gyuyoung Kim
no flags Details
patch (1.79 KB, patch)
2015-05-12 22:19 PDT, daegyu.lee@navercorp.com
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2015-05-12 00:44:33 PDT
I don't know when this feature doesn't work correctly though, I'm sure we have to fix this problem ASAP.

Reproduce site : http://robnyman.github.io/fullscreen/
Comment 1 Gyuyoung Kim 2015-05-12 00:48:46 PDT
Created attachment 252948 [details]
screenshot
Comment 2 daegyu.lee@navercorp.com 2015-05-12 19:39:29 PDT
I've checked that this issue depends on 139109.
Without 139109 patch, Fullscreen feature doesn't work correctly on MiniBrowser.
Comment 3 daegyu.lee@navercorp.com 2015-05-12 19:39:55 PDT
Sorry,
Without 139109 patch, Fullscreen feature works correctly on MiniBrowser.
Comment 4 Gyuyoung Kim 2015-05-12 19:57:20 PDT
(In reply to comment #3)
> Sorry,
> Without 139109 patch, Fullscreen feature works correctly on MiniBrowser.

Oh, this bug is a regression caused by r176631. Thank you for finding it !.

Anyway, Daegyu, will you upload a patch to fix this issue ? If not, I will do.
Comment 5 daegyu.lee@navercorp.com 2015-05-12 22:19:39 PDT
Created attachment 253021 [details]
patch
Comment 6 Gyuyoung Kim 2015-05-12 22:47:31 PDT
Comment on attachment 253021 [details]
patch

IIRC, if m_hadUserInteraction is true, I made this function returns false as below,

bool PageViewportController::updateMinimumScaleToFit(bool userInitiatedUpdate)
{
    if (m_viewportSize.isEmpty() || m_contentsSize.isEmpty() || !m_initiallyFitToViewport || m_hadUserInteraction)
        return false;
  
In this case, however, applyScaleAfterRenderingContents() sometimes needs to be called when m_hadUserInteraction is false. Daegyu, did you run DISABLED_ewk_view_scale_with_fixed_layout() in test_ewk2_view.cpp ? I wonder this patch can fix the disabled API test as well. 

LGTM. r=me.
Comment 7 daegyu.lee@navercorp.com 2015-05-13 01:27:11 PDT
I've checked the DISABLED_ewk_view_scale_with_fixed_layout() is failed with or without this patch.
Comment 8 Gyuyoung Kim 2015-05-13 01:29:14 PDT
(In reply to comment #7)
> I've checked the DISABLED_ewk_view_scale_with_fixed_layout() is failed with
> or without this patch.

Thank you for the check.
Comment 9 WebKit Commit Bot 2015-05-13 02:15:08 PDT
Comment on attachment 253021 [details]
patch

Clearing flags on attachment: 253021

Committed r184283: <http://trac.webkit.org/changeset/184283>
Comment 10 WebKit Commit Bot 2015-05-13 02:15:12 PDT
All reviewed patches have been landed.  Closing bug.