606936 - userTypedValue sticks around after session restore (favicon and star button missing)

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Alice0775 White]

Build Identifier: 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101024 Firefox/4.0b8pre ID:20101024042142

See forum http://forums.mozillazine.org/viewtopic.php?p=10055099#p10055099,

If there are tabs that are not bookmarked,
Star button is missing after session restore.

It appears on reload the tab.


Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open non bookmarked url and bookmarked url
3. Rrestart browser


Actual Results:
 Star button is missing.

Expected Results:
 Star button should not be missing.


Regression Window:
Works:
http://hg.mozilla.org/mozilla-central/rev/dafb1d54cf74
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101022 Firefox/4.0b8pre ID:20101022170624
Fails:
http://hg.mozilla.org/mozilla-central/rev/da395a53dc39
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101022 Firefox/4.0b8pre ID:20101022180939
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dafb1d54cf74&tochange=da395a53dc39

Comment 1

[Alice0775 White]

in addiotion to the comment #0,
> If there are tabs that are not bookmarked,
Even if there is not the above condition, the problem occurs after switching tab.

Comment 2

[Alice0775 White]

Attachment: screenshot

Comment 3

[Siddhartha Dugar [:sdrocking]]

Also note the icon in the site identity button is missing.

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

As the pushlog above made clear, this is a regression from bug 579868.

The problem is that we call into _collectTabData at startup (we've opened new tabs so we trigger a save... something we probably shouldn't do, but we'll take care of some other time). At this point, (with some exception) all the tabs have _tabStillLoading on them AND since we set userTypedValue in restoreHistoryPrecursor when setting the tabs up, we now think the tab should have a userTypedValue and we reset userTypedClear to 0 (because it hasn't actually changed on the browser). So when we get to properly restoring this tab, we end up setting userTypedValue to exactly the value of the page that gets loaded.

I think the right solution here is actually going to be to backout bug 579868 and fix that differently.

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> I think the right solution here is actually going to be to backout bug 579868

The userTyped* stuff, right?

Comment 7

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to comment #6)
> (In reply to comment #5)
> > I think the right solution here is actually going to be to backout bug 579868
> 
> The userTyped* stuff, right?

I'm thinking the whole thing. I sort of like a different solution better now - it would fix the pinning, we would ignore the userTyped* stuff, and it would also start to set up for the fix to 601955.

Comment 8

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

I guess instead of backing out explicitly, I could undo the changes and add the other way of doing it... (I guess in that bug, not here?). The test is mostly good, just need to take out the userTyped* stuff

Comment 9

[Dão Gottwald [:dao]]

How about just ripping out the userTyped* changes and filing a new bug for better tracking of the pinned property? Or maybe handle that altogether in bug 601955?

Either way, backing out the changes related to the pinned property doesn't make much sense to me. After all, we should be better off now than we were before.

Comment 10

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

Remove the relevant userTyped* stuff added in bug 579868. I left part of the userTyped* in - the part where it gets cleared if it isn't set - because that actually makes sense to have and isn't causing this bug.

Comment 12

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/mozilla-central/rev/ef7171b16413