655273 - Back button right-click menu doesn't have page title for history entries created by pushState

Status: VERIFIED FIXED

(Firefox :: Menus, defect)

Description

[Justin Lebar (not reading bugmail)]

STR:

 * http://www.20thingsilearned.com
 * Click to view first page of forward.
 * Navigate forward to the second page of the forward.
 * Right-click on back button.

Expected results:

 * Resulting menu has two entries titled "20 Things I Learned About Browsers and the Web".

Actual results:

 * Resulting menu has one entry titled "20 Things..." and another entry with a URL.

A favicon is also missing, but I've filed bug 655270 on that.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can you please tell us which version of Firefox you are using? Also is that a regression from bug 500328?

Comment 2

[Justin Lebar (not reading bugmail)]

(In reply to comment #1)
> Can you please tell us which version of Firefox you are using?

I'm on Mac 10.6 trunk.

> Also is that a regression from bug 500328?

I doubt it ever worked, but I haven't checked.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Just tested and before the fix on bug 500328 has been landed the title was visible in the back button. Means it is a regression from Firefox 3.6.

Comment 4

[Justin Lebar (not reading bugmail)]

The page I linked doesn't use pushState if pushState isn't there; I think that's why you're seeing that it "works" in FF 3.6.

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Testcase

This simple testcase illustrates the problem.

Notice that it wouldn't have done anything before we had pushstate, pre 4.0.

Comment 6

[Alice0775 White]

If I have not misread STR in comment #0,
UA spoofing is helps.And title and favicon appears in Back/Forward popup menu.
"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Minefield/4.0b5pre"

Regression window(cached m-c hourly):
Works:
http://hg.mozilla.org/mozilla-central/rev/a6c18a123fbb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Minefield/4.0b5pre ID:20100827075702
Fails:
http://hg.mozilla.org/mozilla-central/rev/cf4d7946e2e0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre ID:20100827101721
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a6c18a123fbb&tochange=cf4d7946e2e0
Triggered by:
2914a4cdd7e2	Dan Witte — Bug 588874 - Replace Minefield with Firefox in UA string. r=khuey, sr=jst, a=beta5+
So I think bug 655273 and bug 655270 are TE.



However, Another Bug I found as follows.
Only entry page is registered in the popup, No entry is registered for each page.
Should I filed a bug?

Regression window:
Works, Entry of each page are registered in the back/Forward popup
http://hg.mozilla.org/mozilla-central/rev/6c1a5f4eb350
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110329 Firefox/4.2a1pre ID:20110330085433
Fails, Only front page is registered in the back/Forward popup
http://hg.mozilla.org/mozilla-central/rev/422bbd8245a7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110331 Firefox/4.2a1pre ID:20110331030432
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6c1a5f4eb350&tochange=422bbd8245a7

Comment 7

[Justin Lebar (not reading bugmail)]

> So I think bug 655273 and bug 655270 are TE.

What does "TE" mean?

Changeset 2914a4cdd7e2 is, as you noticed, a UA string change.  So before that change, I bet the website is doing something different -- it's probably not using history.pushState at all.

Were you spoofing the UA when you bisected the second time?

I think it's much more sensible to bisect on the testcase I attached rather than on the website in comment 0, since that site is clearly doing complicated things.  But note that when we initially landed bug 500328, we used the title parameter in pushState (now it's completely ignored), so you'd *still* get different behavior.

I think at this point whether or not this is a regression is academic.  I think I can fix it, no problem, so I don't need any more bisections right now.

Thanks!

Comment 8

[Alice0775 White]

TE means Tech Evangelism.

Anyway, I filed a Bug 655328 for the another bug of comment#6.

Comment 9

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

Comment 10

[Justin Lebar (not reading bugmail)]

http://hg.mozilla.org/mozilla-central/rev/355db79ef6b6

Comment 11

[Justin Lebar (not reading bugmail)]

And backed out, because the test added here was failing:

shistory is null at chrome://mochitests/content/chrome/docshell/test/chrome/test_bug655273.html:31

It works for me, but I only ran the test with TEST_PATH set.  Perhaps if I run the test as part of the larger suite, it'll fail.

Comment 12

[Justin Lebar (not reading bugmail)]

Attachment: Patch v2

Changed the test to a browser-chrome test.  Seems to work better this way...

Comment 13

[Justin Lebar (not reading bugmail)]

http://hg.mozilla.org/mozilla-central/rev/aad0fbc6ae77

Comment 14

[Simona Badau]

Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110527 Firefox/7.0a1

Verified issue using the STR from Comment 0 and the testcase on: Win XP, Win 7, Mac OS X, Ubuntu.