307178 - Fastback(bfcache) breaks visited links marking if frame page

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

1. See frame page.
(e.g., http://www.cgl.ucsf.edu/chimera/docs/ProgrammersGuide/Examples/ )
2. Select "Show Only This Frame" in any frame.
3. Back to frame page by back button.
4. Click left frame's link.

The clicked link isn't marked to visited. This problem is only occured with
Fastback(bfcache).

Comment 1

[Hideo Oshima]

I can reproduce on Trunk/Linux.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20050906 Firefox/1.6a1

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Brian, this is happening because subframe documents don't get OnPageShow
notifications. Is that the expected behaviour? Can you suggest how I can detect
that the subframe document is alive again?

Comment 3

[Hideo Oshima]

Is this depend on bug 274784?

Comment 4

[Brian Ryner (not reading)]

(In reply to comment #2)
> Brian, this is happening because subframe documents don't get OnPageShow
> notifications. Is that the expected behaviour? Can you suggest how I can detect
> that the subframe document is alive again?

This is not the expected behavior -- pageshow events should be firing.  It's possible that this is caused by bug 312117, if it's trunk-only.

Comment 5

[Brian Ryner (not reading)]

It's not that, this happens on the branch as well.

Comment 6

[Brian Ryner (not reading)]

Attachment: patch

We have to clear mEODForCurrentDocument as we start the fake loads for subframes, or they won't notify the content viewer in EndPageLoad, which in turn won't call nsDocument::OnPageShow.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 201490 [details] [diff] [review]
patch

Seems reasonable.  Probably worth getting in on branch too...

Comment 8

[Brian Ryner (not reading)]

checked in on trunk.

Comment 9

[Brian Ryner (not reading)]

Comment on attachment 201490 [details] [diff] [review]
patch

Requesting rc2 approval.  I think this fix is really safe, and it addresses a noticeable issue for users who use link coloring to figure out where they've been.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's not just that, right? Is it not also an API issue for Web developers who expect to receive these events?

Comment 11

[Brian Ryner (not reading)]

That too.

Comment 12

[Mike Schroepfer]

Can we get QA verification on the trunk for this.

Comment 13

[Mike Schroepfer]

I'll do the verify today.  

Comment 14

[Mike Schroepfer]

Comment on attachment 201490 [details] [diff] [review]
patch

Approved r.e. discussion in bug triage today.

Comment 15

[Brian Ryner (not reading)]

Checked in on the branch.