554155 - history.pushState may fire more than one onLocationChange event

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Justin Lebar (not reading bugmail)]

Splitting off from bug 550565.

nsDocShell::AddState contains an explicit call to FireLocationChange, which is always called.  Additionally, AddState sometimes calls SetCurrentURI, which also contains a call to FireOnLocationChange.

We should only fire that event once for a given pushState call.

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

This has the code which I took out from my latest attachment to bug 550565, plus a testcase.

Comment 2

[Justin Lebar (not reading bugmail)]

This patch modifies the same makefile as the patch in bug 550565, so it'll apply with conflicts unless you also have that patch applied.

Comment 3

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1.1

Gets rid of some warnings in the test.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 434285 [details] [diff] [review]
Patch v1.1


>     if (maxStateObjSize < 0)
>-      maxStateObjSize = 0;
>+        maxStateObjSize = 0;


Nit, I'd prefer
if (maxStateObjSize < 0) {
    maxStateObjSize = 0;
}

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Patch for checkin

Comment 6

[[e-mail]]

Comment on attachment 435516 [details] [diff] [review]
Patch for checkin

>+    gBrowser.addTabsProgressListener(listener, Components.interfaces.nsIWebProgress.NOTIFY_ALL);
Is there a reason you need to listen to all location changes in all tabs?

Hmm, I guess you can't easily add the progress listener to the tab's linked browser because that requires you to fiddle around with weak references.

And adding the progress listener to the tabbrowser itself means making that tab the current tab, which isn't ideal either, I guess...

Comment 7

[Justin Lebar (not reading bugmail)]

Attachment: Patch for checkin (rebased)

Rebasing this patch against today's head.  Just changed some context in the Makefile.

Comment 8

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/02e1305294bd