Bug 94123 – [BlackBerry] Reload valid page from Error Page keeps history ViewState and zoom.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Konrad Piascik]

[BlackBerry] Reload valid page from Error Page keeps history ViewState and zoom.

[Konrad Piascik]

Created attachment 158613 [details]
Patch

[Rob Buis]

Comment on attachment 158613 [details]
Patch

Looks good.

[Konrad Piascik]

Created attachment 158617 [details]
Patch

[Antonio Gomes]

Comment on attachment 158617 [details]
Patch

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

It looks ok. I have some questions.

Also , patch does not seem to apply against ToT

> Source/WebKit/blackberry/Api/WebPage.cpp:-1101
> -                m_userScalable = m_webSettings->isUserScalable();
> -                resetScales();

could you explain this part in the changelog?

> Source/WebKit/blackberry/Api/WebPage.cpp:1781
> +    if (m_mainFrame && m_mainFrame->loader() && m_mainFrame->loader()->shouldRestoreScrollPositionAndViewState()
> +            && m_mainFrame->loader()->client() && m_mainFrame->loader()->client()->shouldRestoreViewState())

I think you have extra 4-spaces on the bottom line.

> Source/WebKit/blackberry/Api/WebPage.cpp:1905
> +    // This checks to make sure we're not calling updateViewportSize
> +    // during WebPagePrivate::init().
> +    if (m_backingStore)
> +        updateViewportSize();

I would have done this chunk on it own patch.

> Source/WebKit/blackberry/Api/WebPage.cpp:3593
> +    m_userScalable = m_webSettings->isUserScalable();
> +    resetScales();

could you explain this part in the changelog?

[Konrad Piascik]

Comment on attachment 158617 [details]
Patch

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

>> Source/WebKit/blackberry/Api/WebPage.cpp:-1101
>> -                resetScales();
> 
> could you explain this part in the changelog?

This is explained in the ChangeLog.
"Have the minimum, maximum and initial scale reset whenever the
 viewport properties are changed.  We also reset user scalable at this
 time.  We need to do this in the case of the error page since it's
 scales and user scalable would persist if you refresh and get a valid
 page."

>> Source/WebKit/blackberry/Api/WebPage.cpp:1905
>> +        updateViewportSize();
> 
> I would have done this chunk on it own patch.

This was never an issue until I moved the call site of resetScales()

>> Source/WebKit/blackberry/Api/WebPage.cpp:3593
>> +    resetScales();
> 
> could you explain this part in the changelog?

This is explained in the ChangeLog. see above

[Konrad Piascik]

Created attachment 158843 [details]
Patch

[Antonio Gomes]

Comment on attachment 158843 [details]
Patch

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

> Source/WebKit/blackberry/ChangeLog:13
> +        the user scalalbe flag if we're reloading from an error page.

scalable*

> Source/WebKit/blackberry/ChangeLog:24
> +        new memeber variable m_shouldRestoreViewState based on the ViewState's

member

> Source/WebKit/blackberry/Api/WebPage.cpp:1908
> -    updateViewportSize();
> +    // This checks to make sure we're not calling updateViewportSize
> +    // during WebPagePrivate::init().
> +    if (m_backingStore)
> +        updateViewportSize();

lets move this check to within ::updateViewportSize?

[Konrad Piascik]

Created attachment 158869 [details]
Patch

Fixed requested changes

[WebKit Review Bot]

Comment on attachment 158869 [details]
Patch

Clearing flags on attachment: 158869

Committed r125803: <http://trac.webkit.org/changeset/125803>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.