292933 - Info. popup box on tinderbox display wrong details

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Jaime Mitchell (use [e-mail] for email)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050504 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050504 Firefox/1.0+

Information in the popup box is always displayed for previously selected item

Reproducible: Always

Steps to Reproduce:
1. Set browser.sessionhistory.max_viewers to 50
2. Go to Tinderbox page
3. Select a name under guilty
4. Select Last checkin
5. Go back
6. Select a different name

Actual Results:  
Details for the previously selected name are displayed

Expected Results:  
Details for the new person selected are displayed

Setting browser.sessionhistory.max_viewers to 0 gives expected behaviour.

Comment 1

[Fred Collier]

Mozilla/5.0 (Windows; U; Win95; en-US; rv:1.8b2) Gecko/20050505 Firefox/1.0+ Beast

WFM - browser.sessionhistory.max_viewers set to 10

Comment 2

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

I have viewers set to 5, and I see the problem as reported.  Also I note that
the 'link' is not being marked as 'read', takes a page refresh to update the
link to the status of 'read'. 

Comment 3

[Chase Phillips]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050504
Firefox/1.0+

I can confirm this with the 20050504 Windows trunk build and saw the problem
while using bfcache builds prior to bfcache landing on the trunk.  My
browser.sessionhistory.max_viewers config setting is 5.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

It looks like what's happening here is that the frame for the details window
gets put in the session history cache. Since we only look up subframes' in the
cache by the frame number, when we attempt to find the new details page in the
cache, the frame number is the same and we get the old cache entry. Since we
never check the returned history entry's URI against the requested URI, we
happily use its content viewer, leading to the wrong page being loaded.

I'm not sure why this works with bfcache off, do we cache more aggressively with
it on, perhaps?

Comment 5

[Brian Ryner (not reading)]

Attachment: don't leave mLSHE hanging around

I'm not sure exactly why mLSHE ends up being non-null here, but in any case, it
should be nulled out when restoring a presentation.  This makes sure that
GetChildSHEntry returns null, which it's supposed to when clicking on a
tinderbox popup link.

Comment 6

[Brian Ryner (not reading)]

Comment on attachment 185201 [details] [diff] [review]
don't leave mLSHE hanging around

darin offered to take a look at this.

Comment 7

[Darin Fisher]

Comment on attachment 185201 [details] [diff] [review]
don't leave mLSHE hanging around

Please add a detailed comment explaining why this is being done.

r+sr=darin

Comment 8

[Brian Ryner (not reading)]

Comment on attachment 185201 [details] [diff] [review]
don't leave mLSHE hanging around

requesting approval for this fastback-only fix

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 185201 [details] [diff] [review]
don't leave mLSHE hanging around

a=me with comment (too bad we can't fix the cybercrud names -- L is for
Loading, we gather) for 1.8b3.

/be

Comment 10

[Brian Ryner (not reading)]

checked in

Comment 11

[Boris Zbarsky [:bzbarsky]]

r=bzbarsky, fwiw (just got out of plane-stuff, finally).