Multiple History items when visiting links and using 'Back'

VERIFIED FIXED in Firefox 36

Status

()

VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: jmjeffery, Assigned: smaug)

Tracking

({regression})

36 Branch
mozilla36
x86_64
Windows 7
regression
Points:
---

Firefox Tracking Flags

(firefox35 unaffected, firefox36+ fixed)

Details

(Whiteboard: [testday-20150123], URL)

Attachments

(1 attachment, 5 obsolete attachments)

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
I don't think I did the range command correctly there are too many csets, and showing up through today....

Comment 3

4 years ago
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
Thanks Alice, both of those bugs are 'security bugs' , and I can't access them.
Keywords: regressionwindow-wanted

Comment 5

4 years ago
wait
status-firefox35: --- → unaffected
status-firefox36: --- → affected
tracking-firefox36: --- → ?
Flags: needinfo?(kinetik)

Updated

4 years ago
Flags: needinfo?(kinetik)

Comment 6

4 years ago
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
Blocks: 855443
Component: Bookmarks & History → Document Navigation
Product: Firefox → Core
Version: Trunk → 36 Branch
Thanks Alice.


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

Investigating.
Assignee: nobody → bugs
(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.
(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
Created attachment 8516771 [details] [diff] [review]
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.
Attachment #8516200 - Attachment is obsolete: true
Attachment #8516639 - Attachment is obsolete: true
Created attachment 8516877 [details] [diff] [review]
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.
Attachment #8516771 - Attachment is obsolete: true
Attachment #8516877 - Flags: review?(bzbarsky)
Comment on attachment 8516877 [details] [diff] [review]
v7 + a test

Cancelling review. Looks like there is still some issue :/
Attachment #8516877 - Flags: review?(bzbarsky)
tracking-firefox36: ? → +
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.  :(
Attachment #8516991 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/495347922a50
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox36: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Verifying Fixed using latest hourly win32 m-c build from cset:
https://hg.mozilla.org/mozilla-central/rev/acbd7b68fa8c
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify]
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0

Verified fixed.
Whiteboard: [testday-20150123]
You need to log in before you can comment on or make changes to this bug.