PageViewportController::m_rawAttributes.devicePixelRatio is not initialized

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Yael]

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.

[Yael]

Created attachment 169458 [details]
Patch

[WebKit Review Bot]

Comment on attachment 169458 [details]
Patch

Clearing flags on attachment: 169458

Committed r131802: <http://trac.webkit.org/changeset/131802>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Kenneth Rohde Christiansen]

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

[Andras Becsi]

(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.

[Yael]

(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)

[Andras Becsi]

(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.