Bug 30424

Summary:

REGRESSION (r48687): Pages on ucas.com appear blank

Product:

WebKit

Reporter:

Fumitoshi Ukai <ukai>

Component:

WebCore Misc.

Assignee:

Brady Eidson <beidson>

Status:

RESOLVED FIXED

Severity:

Major

CC:

ap, beidson, chmeeedalf, chrishilbert, darin, mrowe, sam, simon.fraser

Priority:

Keywords:

InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed fix and layout test	darin: review+, beidson: commit-queue-

Description Fumitoshi Ukai 2009-10-15 20:42:40 PDT

WebKit r48687 causes the regression reported on http://crbug.com/24123

Visit the following URLs. It is expected to show login pages, but show only blank page with WebKit r48687 or later.
It works as expected with WebKit r48686.

 https://track.ucas.com/ucastrack/Login.jsp
 https://apply2.ucas.com/appreg2010/SecurityServlet

As investigated Developer Tools in chrome, it doesn't show any body in Element page.
However, view source shows correct html contents.

Comment 1 Mark Rowe (bdash) 2009-10-15 23:31:41 PDT

In the future please describe the bug in the “Summary" field.  If the bug is a regression you should include the REGRESSION tag at the start of the summary, and you should CC the author of the original change on the bug.

Comment 2 Mark Rowe (bdash) 2009-10-15 23:32:28 PDT

<rdar://problem/7308952>

Comment 3 Brady Eidson 2009-10-26 11:13:30 PDT

*** Bug 30704 has been marked as a duplicate of this bug. ***

Comment 4 Brady Eidson 2009-10-26 11:14:59 PDT

In the bugzilla dupes and radar dupes we've seen of this issue, we've seen parsing of the page stop once a schedule redirection is hit.  In a couple cases it's been a history.forward(), and in a couple it's been a <meta> refresh.

That symptom matches quite obviously with the nature of r48687

Comment 5 Brady Eidson 2009-10-26 13:59:56 PDT

Created attachment 41892 [details]
Proposed fix and layout test

Comment 6 Darin Adler 2009-10-26 14:55:56 PDT

Comment on attachment 41892 [details]
Proposed fix and layout test

> +    // http://webkit.org/b/30424 - Invalid history navigations (such as history.forward() during a new load) have
> +    // the side effect of cancelling any scheduled redirects. We also avoid the possibility of cancelling the
> +    // current load by avoiding the scheduled redirection altogether.

I am not a huge fan of including bug URLs in the code. Otherwise, this comment is fine. I'd just leave out the URL.

> +if (window.layoutTestController)
> +    window.layoutTestController.dumpAsText();

The second line can just say "layoutTestController", not "window.layoutTestController".

> +http://webkit.org/b/30424 and <rdar://problem/7308952> - REGRESSION (r48687) - Some pages appear blank.<br>

You need an &lt; here instead of the < sign.

Comment 7 Brady Eidson 2009-10-26 16:21:27 PDT

Committed r50111.
http://trac.webkit.org/changeset/50111

Comment 8 Brady Eidson 2009-10-26 21:41:58 PDT

*** Bug 30746 has been marked as a duplicate of this bug. ***

Comment 9 Simon Fraser (smfr) 2009-10-27 15:48:27 PDT

#webkit is reporting crashes:
http://nopaste.com/p/aeY6lni8fb

Comment 10 Darin Adler 2009-10-27 17:09:15 PDT

Lets put those crashes in a new bug report. Are they reproducible?

Comment 11 Simon Fraser (smfr) 2009-10-27 17:11:56 PDT

They are being tracked via bug 30839.

Comment 12 Brady Eidson 2009-10-27 17:12:57 PDT

There was already a new bugzilla - https://bugs.webkit.org/show_bug.cgi?id=30839

But they are *NOT* fallout from this change.  We have radars for them from weeks ago (<rdar://problem/7313414>)

Sadly, they are not reliably reproducible.

Comment 13 Justin Hibbits 2009-10-27 19:09:34 PDT

Just tested with the nightly 50124, and it works correctly, thanks.