1090918 - Multiple History items when visiting links and using 'Back'

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

Certain sites are creating multiple history items causing the user to click the 'back' button several times to get back to original page.

Example:
1. wishtv.com - read any story - going back takes 2 clicks, right-click on the 'back button' shows two entries for story before main page
2. imdb.com - read any story - going back takes 4-5 click - same as above right-click and notice all the same entries... 

from hourly builds:

20141015110015 7de522bd9785 good
20141015113219 a280a03c9f3c bad

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange7de522bd9785=&tochange=a280a03c9f3c

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

I don't think I did the range command correctly there are too many csets, and showing up through today....

Comment 2

[Jim Jeffery not reading bug-mail 1/2/11]

Correct range:  
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7de522bd9785&tochange=a280a03c9f3c

Comment 3

[Alice0775 White]

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/74fc57a6f5e5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141014173633
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f0732a51004
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141014175933
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=74fc57a6f5e5&tochange=5f0732a51004

Comment 4

[Jim Jeffery not reading bug-mail 1/2/11]

Thanks Alice, both of those bugs are 'security bugs' , and I can't access them.

Comment 5

[Alice0775 White]

wait

Comment 6

[Alice0775 White]

Sorry comment#3 is wrong. I must sleep.


Correct regression window is as follows.

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fdcdc0dbc52c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141015030332
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3e717e9b628
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141015031530
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fdcdc0dbc52c&tochange=e3e717e9b628

Suspect:  Bug 855443

Comment 7

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

Thanks Alice.


Jim, it would be great to have exact steps to reproduce, but I'll try to find such.

Investigating.

Comment 8

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Olli Pettay [:smaug] from comment #7)
> Thanks Alice.
> 
> 
> Jim, it would be great to have exact steps to reproduce, but I'll try to
> find such.
> 
> Investigating.

Ollie, either of the links in comment #0 www.wishtv.com  or imdb. com  produces it easily.
Read any story on either site, then click back button, note for wishtv.com that it takes about 2 clicks to get, imdb.com takes 4-5 

Right-clicking on the 'back' button will show the multiple items in history before you get back to the starting page.

Comment 9

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

Yeah, I can reproduce.

Comment 10

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

Attachment: v3 - wip

https://tbpl.mozilla.org/?tree=Try&rev=d8da4b1c30bb

Comment 11

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Olli Pettay [:smaug] from comment #10)
> Created attachment 8515701 [details] [diff] [review]
> v3 - wip
> 
> https://tbpl.mozilla.org/?tree=Try&rev=d8da4b1c30bb

Just tested this try-build, and it seems to fix the problem on imbd.com and wistv.com sites.  No more multiple history links when going 'back' to main page.

Thanks Olli

Comment 12

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

Attachment: v5

https://tbpl.mozilla.org/?tree=Try&rev=74f9f12db7aa

Comment 13

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

Attachment: v6

about:blank document + document.write + session history is messy :/

https://tbpl.mozilla.org/?tree=Try&rev=68b16ba953f1

Comment 14

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

Attachment: v7

https://tbpl.mozilla.org/?tree=Try&rev=136c4b161f55

And there is a real bug with nested frames. Splitting AddChildSHEntry to
AddChildSHEntry and AddChildSHEntryInternal deals with that.

Comment 15

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

Attachment: v7 + a test

https://tbpl.mozilla.org/?tree=Try&rev=db3a43c34fd0

I should try to write more tests, but at least there is a test for this particular case. The patch gives the same test result as builds without the patch for bug 855443.
And this doesn't, based on the current tryserver results, regress bug 855443.


The patch has the following changes
(1) Backout bug 855443
(2) DoAddChildSHEntry is renamed to AddChildSHEntryToParent to indicate what the method actually does
(3) Split AddChildSHEntry to two parts, AddChildSHEntry and AddChildSHEntryInternal to prevent the internal recursion to deal with mLSHE and mOSHE which are
    really about the original docshell only. We shouldn't look at them higher up in the docshell tree.
    (http://mxr.mozilla.org/mozilla-central/source/dom/html/test/test_iframe_sandbox_inheritance.html?force=1 hit that issue)
(4) When there is mLSHE, we try to replace the existing nsISHEntry in order to fix this particular bug without causing the assertion in bug 855443


And this all is super messy.

Comment 16

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

Comment on attachment 8516877 [details] [diff] [review]
v7 + a test

Cancelling review. Looks like there is still some issue :/

Comment 17

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

Attachment: v8

https://tbpl.mozilla.org/?tree=Try&rev=192e30470fdf

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8516991 [details] [diff] [review]
v8

r=me.  Though my confidence in being able to notice a problem in this patch is pretty low.  :(

Comment 19

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/495347922a50

Crossing fingers. This is super regression risky stuff.

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/495347922a50

Comment 21

[Jim Jeffery not reading bug-mail 1/2/11]

Verifying Fixed using latest hourly win32 m-c build from cset:
https://hg.mozilla.org/mozilla-central/rev/acbd7b68fa8c

Comment 22

[Gabriela [:gaby2300]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0

Verified fixed.