Closed Bug 606936 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox :: Session Restore, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b8
Tracking Status
blocking2.0 --- final+

People

(Reporter: alice0775, Assigned: zpao)

References

Details

(Keywords: regression)

Attachments

(2 files)

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
blocking2.0: --- → ?
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.
Attached image screenshot
Also note the icon in the site identity button is missing.
Component: Location Bar → Session Restore
OS: Windows 7 → All
QA Contact: location.bar → session.restore
Hardware: x86 → All
Summary: Star button is missing after session restore → userTypedValue sticks around after session restore (favicon and star button missing)
Target Milestone: Firefox 4.0 → ---
Duplicate of this bug: 607443
blocking2.0: ? → final+
Assignee: nobody → paul
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.
(In reply to comment #5)
> I think the right solution here is actually going to be to backout bug 579868

The userTyped* stuff, right?
(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.
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
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.
Attached patch Patch v0.1Splinter Review
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.
Attachment #489237 - Flags: review?(dietrich)
Attachment #489237 - Flags: review?(dietrich) → review+
Duplicate of this bug: 611577
Pushed http://hg.mozilla.org/mozilla-central/rev/ef7171b16413
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Duplicate of this bug: 610244
Duplicate of this bug: 612236
Duplicate of this bug: 614553
You need to log in before you can comment on or make changes to this bug.