623829 - Reloading a page with frames breaks history navigation

Status: NEW

(Core :: DOM: Navigation, defect)

Description

[Will Roberts [:bws42]]

User-Agent:       Mozilla/5.0 (X11; Linux x86_64; rv:2.0b9pre) Gecko/20110106 Firefox/4.0b9pre
Build Identifier: 

Grab the testcase from Bug 587654: https://bugzilla.mozilla.org/attachment.cgi?id=466305

1. View the directory containing the files
2. Click on index.htm
3. Right-click and reload the page
4. Click "Click this link"
5. Click the back button
6. You're back at the directory listing (wrong!), now click the forward button.
7. You're at index.htm, now click the forward button again and it won't do anything.

Reproducible: Always

Comment 1

[Will Roberts [:bws42]]

This has the same regression range as Bug 597315:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=116f2046b9ef&tochange=095418dfa2e6

So I'm going to blame Bug 462076.

Comment 2

[Johnny Stenback (:jst)]

Olli, any ideas here? Do we need to block on this?

Comment 3

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

I haven't had time to look at this.

I'm not sure whether this should block. There are no duplicates to this
and bug 462076 landed long ago.

Comment 5

[al_9x]

When you are doing rapid releases and (effectively) forcing upgrades, regressions should have higher priority than this (9.5 months old)

Comment 6

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

This has nothing to do with rapid releases. The patch causing this landed way before we started
to talk about rapid releases.

This is clearly not very often happening problem, since it was noticed about 4 months after
the patch causing the regression landed. And now, 14 months after the patch a new bug
was filed.
Unfortunately session history is tricky and not defined properly in any spec - which leads to
problems in all the browsers.

But anyhow, I'll try to look at this soon. Thank you for reminding me.
(Or feel free to fix this. Firefox is, you know, open source ;) )

Comment 7

[al_9x]

(In reply to Olli Pettay [:smaug] from comment #6)
> This has nothing to do with rapid releases.

The relevance of rapid releases is this:

rapid releases provide an opportunity to both fix and introduce regressions quickly.  The later is automatic, if you don't concentrate on the fix part, rapid releases become a clear liability, accumulating little annoying regressions with each release.  Eventually it becomes a death by a thousand cuts, Fx grows progressively more unpleasant to use.

Because upgrades are effectively forced, testing cycles are short, there is no really stable, well tested release to cling to (like 2.0 was or 3.6 may still be for a time).  The ESR will not address this issue as only major security problems will be fixed, accumulated regressions will remain for its lifetime.

This is a recipe for losing users.  To avoid this, you need to shift the balance toward the advantage of rapid releases by prioritizing clear regressions and rolling out fixes quickly, even when the bugs don't seems individually severe, because it's their accumulation you're trying to avoid.

Comment 8

[Boris Zbarsky [:bzbarsky]]

We would NOT have shipped a fix for this sort of bug in a 3.6.x release.

Furthermore, we would not have held a 3.6.0 or 4.0 release for this bug.

So Olli is right; rapid release is not an issue here.  It does give us an opportunity to fix regressions faster, but it doesn't lead to any sort of "accumulation" that wasn't there before.

Comment 9

[al_9x]

(In reply to Boris Zbarsky (:bz) from comment #8)
> We would NOT have shipped a fix for this sort of bug in a 3.6.x release.
> 
> Furthermore, we would not have held a 3.6.0 or 4.0 release for this bug.
> 
> So Olli is right; rapid release is not an issue here.  It does give us an
> opportunity to fix regressions faster, but it doesn't lead to any sort of
> "accumulation" that wasn't there before.

Each release will add regressions, that's a given, if you don't then remove them quickly enough they will accumulate, inflow > outflow = accumulation.  Opportunities to fix regressions have to be taken advantage of, they weren't here.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Each release will add regressions, that's a given

Yes, and fix bugs and fix regressions.  Again, nothing to do with this bug per se.

Comment 11

[O. Atsushi (Torisugari)]

(In reply to Olli Pettay [:smaug] from comment #6)
> Unfortunately session history is tricky and not defined properly in any spec
> - which leads to problems in all the browsers.

How about clearing sub-frames' SH entries on a LOAD_RELOAD_NORMAL request, no matter which nsISHEntry::HasDynamicallyAddedChild returns true or false? Both testcases (this and bug 697718) are not "dynamic" frames.

I'm quite sure there exists a really buggy part anywhere else (theoretically there's no need to touch SH at all unless the reloading request fails), but this band-aid will hide the bug from users, just like forced-Reload and LOAD_REFRESH requests.

Comment 12

[O. Atsushi (Torisugari)]

When we move
[pageA]->[pageB + frame1]->[pageB + frame2]->[pageC]
and back to
[pageA]<-[pageB + frame1]<-[pageB + frame2]<-[pageC]
and forward to
[pageA]->[pageB + frame1]->[pageB + frame2]->[pageC]


[pageB + frame2]<-[pageC]          is bfcached     ([pageB + frame2]).
[pageA]->[pageB + frame1]          is bfcached     ([pageB + frame1]).
[pageB + frame1]<-[pageB + frame2] is not bfcached ([pageB + frame1]).
[pageB + frame1]->[pageB + frame2] is not bfcached ([pageB + frame2]).

After I reload [pageB + frame1]

backward: [pageA]<-[     skip     ]<-[pageB + frame1]<-[pageC]
forward : [pageA]->[pageB + frame1]->[     skip     ]->[pageC]

Both [pageB + frame1]<-[pageC] and [pageA]->[pageB + frame1] is bfcached.

If browser.sessionhistory.max_total_viewers is 0, and reload [pageB + frame1]

forward : [pageA]->[pageB + frame1]->[     skip     ]->[pageC]
backward: [pageA]<-[pageB + frame1]<-[pageB + frame2]<-[pageC]
forward : [pageA]->[pageB + frame1]->[pageB + frame2]->[pageC]
...
Navigation is broken right after reload, but it recovers automatically.

Comment 13

[Adrian Florinescu [:aflorinescu]]

Still reproducible on version 50.0a1 Build ID 20160613030258.