Bug 42298 – Regression (r63100): m_loadType being used to reset scroll position after being reset

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Nate Chapin]

In http://trac.webkit.org/changeset/63100, I changed FrameLoader::handledOnloadEvents() to reset m_loadType to FrameLoadTypeStandard (to ensure that loads after the load event would not propagate the caching behavior of the original request).  It turns out this solution won't work, since there are some other behaviors that depend on m_loadType after the load event.  Notably, in didFirstLayout(), we decide whether or not to call history()->restoreScrollPositionAndViewState() based on m_loadType, so scroll position restoring for back/forward navigations broke (which caused gtk test to fail).

This change should be fixable by reverting my changes to handledOnloadEvents() and being a little more careful in addExtraFieldsToRequest().

[Nate Chapin]

Created attachment 61577 [details]
Fix + uncomment the gtk test assertion that broke

I don't have a gtk build, but msanchez was kind enough to patch this in and confirm the assertion now passes.

[Mario Sanchez Prada]

(In reply to comment #1)
> Created an attachment (id=61577) [details]
> Fix + uncomment the gtk test assertion that broke
> 
> I don't have a gtk build, but msanchez was kind enough to patch this in and 
> confirm the assertion now passes.

Yes, I confirm both that the assertion passes and that there were no regressions found while re-running the layout tests with 'run-webkit-tests --gtk'

Thanks Nate for fixing this issue

[WebKit Commit Bot]

Comment on attachment 61577 [details]
Fix + uncomment the gtk test assertion that broke

Clearing flags on attachment: 61577

Committed r63528: <http://trac.webkit.org/changeset/63528>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/63528 might have broken Qt Linux Release

[Mario Sanchez Prada]

(In reply to comment #5)
> http://trac.webkit.org/changeset/63528 might have broken Qt Linux Release

Output in the build bot [1] says this for the failing test(fast/dom/Range/range-compareNode.html):

CONSOLE MESSAGE: line 198: TypeError: Result of expression 'layoutTestController.dumpAsText' [undefined] is not a function.

I'm not 100% sure, of course, but I don't see how this patch could get this test failing this way. Hence, I've got the feeling is something else.

Any guessing/idea?


[1] http://build.webkit.org/results/Qt%20Linux%20Release/r63529%20(15543)/fast/dom/Range/range-compareNode-pretty-diff.html