Windows Live support threads continuously reload

RESOLVED FIXED

Status

Tech Evangelism Graveyard
English US
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: TimAbraldes, Assigned: blizzard)

Tracking

Details

Attachments

(1 attachment)

Last good nightly: 2011-04-20
First bad nightly: 2011-04-21

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=45b20f137549&tochan
ge=46fdf12082d4

bug 628069 ?
Component: General → General
Product: Firefox → Core
QA Contact: general → general
Actually, a regression from bug 640387.

I'm guessing this will end up as evang?
Blocks: 640387
Component: General → Document Navigation
QA Contact: general → docshell
Awesome.

Is Windows Live actually using pushstate here?  I doubt it, since IE doesn't support it...
No obvious pushState calls, assuming nsHistory::PushState is the right place to breakpoint.

The page script does have this:

Sys._Application.prototype._raiseNavigate = function () {
....
        if (Sys.Browser.agent === Sys.Browser.Firefox && window.location.hash && (!window.frameElement || window.top.location.hash)) window.history.go(0)

This ends up getting called off DOMContentLoaded.

No idea why bug 640387 would have affected this code, so far.
Is it possible that we used to short-circuit the go(0) load and no longer do so or something?
> This ends up getting called off DOMContentLoaded.

Does _raiseNavigate also get called off hashchange?  That could explain things...
I'm seeing us always enter go(0) off DOMContentLoaded.

So in InternalLoad, I see sameDocument == true, mOSHE == aSHEntry, aSHEntry != 0.  So doShortCircuitedLoad = false.  

But that's purposeful, as the comments say:

8436            // The restriction tha the SHEntries in (a) must be different ensures
8437            // that history.go(0) and the like trigger full refreshes, rather than
8438            // short-circuited loads.

The key difference is that the old code did this:

-        allowScroll = (ourPageIdent == otherPageIdent);
...
-        if (wasAnchor || (sameDocIdent && (mOSHE != aSHEntry))) {

I bet wasAnchor used to be true here....
> I bet wasAnchor used to be true here....

I'm spinning a build to see.
Ah, I'm now running a 3.0 kernel, and something in the old tree doesn't understand.  I'll try on my mac tomorrow.
> I bet wasAnchor used to be true here....

Yep, that's right.  But that's the wrong behavior, right?  It means that history.go(0) doesn't do anything if the page has a hash.  (Which is the behavior this page expects, bizarrely.)
Yeah, the old behavior seems wrong at first glance....

What do other browsers do?
Created attachment 567780 [details]
Testcase

If the page refreshes, the displayed random number will change.

Try pressing the button after loading the testcase, and after appending #foo to the URL.
On the attached testcase:

 * IE9 and Firefox refresh the page regardless of whether there's a hash in the URI.
 * Chrome and Safari refresh the page only if there isn't a hash in the URI.

I think our behavior here is OK, seeing as it matches another browser and is more sane than the previous behavior.
OK, Webkit does what we used to do, looks like (refreshes only when the hash is empty).

Opera does what we do now.

IE9 likewise.

Given that, and given that this page is explicitly running the broken code only in Firefox, I think we should evangelize this.  I'll send mail.
Assignee: nobody → english-us
Component: Document Navigation → English US
Product: Core → Tech Evangelism
QA Contact: docshell → english-us
Version: Trunk → unspecified
(Assignee)

Updated

6 years ago
Assignee: english-us → blizzard
Seems fixed.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Product: Tech Evangelism → Tech Evangelism Graveyard
You need to log in before you can comment on or make changes to this bug.