Bug 99751 - PageViewportController::m_rawAttributes.devicePixelRatio is not initialized
Summary: PageViewportController::m_rawAttributes.devicePixelRatio is not initialized
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-18 13:28 PDT by Yael
Modified: 2012-10-19 05:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.72 KB, patch)
2012-10-18 13:37 PDT, Yael
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yael 2012-10-18 13:28:01 PDT
PageViewportController::m_rawAttributes.devicePixelRatio is not initialized and can cause the zoom level to become infinity.
It seems in qt port it starts with garbage value but in EFL port starts with 0, and then the zoom level is set to infinity in computeMinimumScaleFactorForContentContained . Once minimum scale starts as infinity, it never goes down.
Comment 1 Yael 2012-10-18 13:37:58 PDT
Created attachment 169458 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-18 14:20:20 PDT
Comment on attachment 169458 [details]
Patch

Clearing flags on attachment: 169458

Committed r131802: <http://trac.webkit.org/changeset/131802>
Comment 3 WebKit Review Bot 2012-10-18 14:20:23 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Kenneth Rohde Christiansen 2012-10-19 01:34:08 PDT
Comment on attachment 169458 [details]
Patch

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

> Source/WebKit2/UIProcess/PageViewportController.cpp:74
> +    m_rawAttributes.devicePixelRatio = 1;

ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else
Comment 5 Andras Becsi 2012-10-19 02:08:07 PDT
(In reply to comment #4)
> (From update of attachment 169458 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review
> 
> > Source/WebKit2/UIProcess/PageViewportController.cpp:74
> > +    m_rawAttributes.devicePixelRatio = 1;
> 
> ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else

Since the recent refactor, we do not use m_rawAttributes.devicePixelRatio any more, so this patch does not really have an effect.

I'll prepare a patch to remove it from the attributes.
Comment 6 Yael 2012-10-19 05:17:25 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 169458 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review
> > 
> > > Source/WebKit2/UIProcess/PageViewportController.cpp:74
> > > +    m_rawAttributes.devicePixelRatio = 1;
> > 
> > ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else
> 
> Since the recent refactor, we do not use m_rawAttributes.devicePixelRatio any more, so this patch does not really have an effect.
> 
> I'll prepare a patch to remove it from the attributes.

Well, without this patch, I saw a black screen in my version of EFL MIniBrowser, after hooking it up with PageViewController.
The reason was that the zoom level was infinity.

m_rawAttributes.devicePixelRatio is actually used in computeMinimumScaleFactorForContentContained (ViewportAgruments.cpp)
Comment 7 Andras Becsi 2012-10-19 05:27:51 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (From update of attachment 169458 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=169458&action=review
> > > 
> > > > Source/WebKit2/UIProcess/PageViewportController.cpp:74
> > > > +    m_rawAttributes.devicePixelRatio = 1;
> > > 
> > > ARgh, this device pixel ratio should be removed from here instead and we should use Page::deviceScaleFactor like everyone else
> > 
> > Since the recent refactor, we do not use m_rawAttributes.devicePixelRatio any more, so this patch does not really have an effect.
> > 
> > I'll prepare a patch to remove it from the attributes.
> 
> Well, without this patch, I saw a black screen in my version of EFL MIniBrowser, after hooking it up with PageViewController.
> The reason was that the zoom level was infinity.
> 
> m_rawAttributes.devicePixelRatio is actually used in computeMinimumScaleFactorForContentContained (ViewportAgruments.cpp)

Yes, you are right restrictMinimumScaleFactorToViewportSize and computeMinimumScaleFactorForContentContained could be called before didChangeViewportAttributes.