Not all code paths lead to the fallback viewport arguments being used. Ensure that the fallback ViewportArguments for all loads are used if no meta viewport is present.
Created attachment 138592 [details] Patch
This looks much cleaner. Since this is actually an implementation of the original bug then I'm inclined to re-open the previous bug, revert the old commit and cleanup this change to be correct. I'm just not sure if its worth it. That being said, I'd like to see if there's some way to write a unit test for this.
Comment on attachment 138592 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=138592&action=review > Source/WebKit/blackberry/Api/WebPage.cpp:3158 > - currentViewportArguments.targetDensityDpi = deviceDPI; > + m_viewportArguments.targetDensityDpi = deviceDPI; > } we set 'deviceDPI' here... > Source/WebKit/blackberry/Api/WebPage.cpp:3160 > - ViewportAttributes result = computeViewportAttributes(currentViewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); > + ViewportAttributes result = computeViewportAttributes(m_viewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); ... and pass it as a parameter here. Is it really needed?
(In reply to comment #3) > (From update of attachment 138592 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=138592&action=review > > > Source/WebKit/blackberry/Api/WebPage.cpp:3158 > > - currentViewportArguments.targetDensityDpi = deviceDPI; > > + m_viewportArguments.targetDensityDpi = deviceDPI; > > } > > we set 'deviceDPI' here... > > > Source/WebKit/blackberry/Api/WebPage.cpp:3160 > > - ViewportAttributes result = computeViewportAttributes(currentViewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); > > + ViewportAttributes result = computeViewportAttributes(m_viewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); > > ... and pass it as a parameter here. Is it really needed? Yes this is needed.
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 138592 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=138592&action=review > > > > > Source/WebKit/blackberry/Api/WebPage.cpp:3158 > > > - currentViewportArguments.targetDensityDpi = deviceDPI; > > > + m_viewportArguments.targetDensityDpi = deviceDPI; > > > } > > > > we set 'deviceDPI' here... > > > > > Source/WebKit/blackberry/Api/WebPage.cpp:3160 > > > - ViewportAttributes result = computeViewportAttributes(currentViewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); > > > + ViewportAttributes result = computeViewportAttributes(m_viewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); > > > > ... and pass it as a parameter here. Is it really needed? > > Yes this is needed. Could you explain why? Why not assign it from within ::computeViewportAttributes?
View in context: https://bugs.webkit.org/attachment.cgi?id=138592&action=review >> Source/WebKit/blackberry/Api/WebPage.cpp:3160 >> + ViewportAttributes result = computeViewportAttributes(m_viewportArguments, desktopWidth, deviceWidth, deviceHeight, deviceDPI, m_defaultLayoutSize); > > ... and pass it as a parameter here. Is it really needed? We're not changing the DeviceDPI, what we're doing is changing the default targetDensityDPI of the meta viewport arguments. On the PlayBook the targetDensityDPI if not specified should be the DeviceDPI. The DeviceDPI passed into the computeViewportAttributes needs to be the correct deviceDPI otherwise there will be no scaling.
Created attachment 138601 [details] Patch
Comment on attachment 138601 [details] Patch Clearing flags on attachment: 138601 Committed r115135: <http://trac.webkit.org/changeset/115135>
All reviewed patches have been landed. Closing bug.