Closed Bug 293588 Opened 18 years ago Closed 18 years ago

Wrong items are selected in dropdowns

Categories

(Core :: DOM: Navigation, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: bugzilla, Assigned: bzbarsky)

References

Details

(Keywords: dataloss, regression)

Attachments

(2 files, 1 obsolete file)

Build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050509
Firefox/1.0+

Since some days back I noticed that the wrong elements are selected in a
dropdown. This has caused me to change all fields in certain bugs when cc:ing
myself on them.

Steps to reproduce:
1. Go to https://bugzilla.mozilla.org/show_bug.cgi?id=293446
2. Now change the url so that it goes to 915 and press enter
3. Notice that its not core/layout tables in the bug, its core /build config
4. Doing a shift + reload shows the correct values
cc bzbarsky
Attached image Screenshot
This works fine in 20050504, broken in 20050505, which makes bug 274784 a
suspect

Note that i haven't enabled the "fastback/bfcache" feature
OS: Windows XP → All
Hardware: PC → All
Note that step 2 means change the URL in the location bar, not in the bug...)

After step 2, I saw the expected results, but then I went back, and then forward
again, and voila! The bug was Layout: Build Config...

Sometimes I have to go back and forwards more than once to reproduce. I have
bfcache enabled with 5 viewers.
This is probably the same basic issue as bug 293135 -- the form state is being
placed in the wrong (new) history entry.  Please retest in today's builds; it
should be fixed.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050510
Firefox/1.0+
I still see this issue with the beast build I've been using since the landing of
bug 293135. Often when I go to a bug the bug is tagged as Firefox/Bookmarks but
when I ctrl-f5 the page the correct Component is shown.
Same here. With todays build (20050510 ) i can still reproduce everytime
following the steps in comment 0
bryner, do you have any idea what's up here?
Assignee: nobody → bryner
Keywords: dataloss
Flags: blocking1.8b2?
Blocks: 293793
I'm not able to reproduce this problem on any platform, as of 2005-05-11.  Can
you provide any additional details that might help here?  Does the problem
happen with a new profile?
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050511
Firefox/1.0+ ID:2005051111

I still see this bug.

1. Create a new profile.
2. Go to https://bugzilla.mozilla.org/show_bug.cgi?id=293446
3. Accept the secure site dialog ("OK")
4. Manually edit the URL bar so the bug number points to bug 293123
5. Hit Return to load the new bug page in the current tab.
6. Note that bug 293123 says it's in Core / Build Config (Wrong) - Note that
Version: Branch (Wrong)
7. Do a CTRL-F5
8. Note that bug 293123 now says it's in Core / Security: UI (Correct) - Note
that Version: Trunk (Correct)
FWIW as the page is rendering I see a glimpse of "Component:" set to the right
value (Security:UI) but then when the page finishes it changes to "Build: Config".

Setting my browser.history_expire_days to 0 doesn't change the misbehaviour.
martijn, can you reproduce this by any chance?  If so, could you possibly create
a testcase?  bryner and I are both failing to reproduce this... :(
I was thinking about this...  GetScriptGlobalObject() will return non-null for a
document if it has an mDocumentContainer even if mScriptGlobalObject is null. 
So it may be that UnbindFromTree on the old document is still storing state in
the new document's SHEntry...
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050511
Firefox/1.0+ ID:2005051111

OK, with a new profile and comment 9 repo steps I get the bug.
With a new profile and browser.sessionhistory.max_viewers=0 and the comment 9
repo steps I still get the bug.
But with a new profile and browser.sessionhistory.max_viewers=1 the bug
(incorrect item select in dropdown boxes) goes away.
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050511
Firefox/1.0+ ID:2005051108

i can still repro this with
browser.sessionhistory.max_viewers 0 (bfcache=off!)

i can't with bfcache on (any non-0 value)
http://web.mit.edu/bzbarsky/www/testcases/shistory/formState.html is a minimal
testcase that shows what I think is the problem (with a text input, but the idea
is the same).  Definitely caused by what I said in comment 12...
Attached patch Partial patch (obsolete) — Splinter Review
This fixes my testcase, but it's not quite right, I think.  See the XXX
comments in the last hunk for details.

Note also that printing messes with the layout history state as well (in
ReflowPrintObject()); I strongly suspect that this shares the docshell with the
non-printing presentation, and I'm not quite sure what the ordering of that
code is with everything else we have going on....
Note that my testcase has security implications that I feel should make fixing
this a blocker for anything we want to release.  I can explain those in this bug
or in a separate bug, as desired...
Flags: blocking-aviary1.1?
Blocks: 293709
Ok, the XXX-marked code is called from exactly two places:

1)  Printing.
2)  nsDocumentViewer::Hide, if the viewer is non-sticky.

I _think_ we might be ok, since people generally wouldn't be calling Hide() on a
non-current viewer (and if they were, they already had problems)....

bryner, thoughts?

Comment on attachment 183357 [details] [diff] [review]
Partial patch

I think this is ok for now; for 1.9 we may want to rethink the entire
ownership/etc model for the layout state...
Attachment #183357 - Flags: superreview?(bryner)
Attachment #183357 - Flags: review?(bryner)
Comment on attachment 183357 [details] [diff] [review]
Partial patch

If you no longer feel that the XXX-marked code is "definitely a problem with
bfcache on", then please update the comment to reflect that.  Looks good
otherwise, and should fix the Tp regression as well.  Should the check I added
to nsGeneircHTMLFormElement::UnbindFromTree, to make sure there is a
ScriptGlobalObject, be removed?
Attachment #183357 - Flags: superreview?(bryner)
Attachment #183357 - Flags: superreview+
Attachment #183357 - Flags: review?(bryner)
Attachment #183357 - Flags: review+
> then please update the comment to reflect that

Will do.

> Should the check I added
> to nsGeneircHTMLFormElement::UnbindFromTree, to make sure there is a
> ScriptGlobalObject, be removed?

Yes.  Will do that too.
Comment on attachment 183357 [details] [diff] [review]
Partial patch

requesting approval to check this in
Attachment #183357 - Flags: approval1.8b2?
Attachment #183357 - Attachment is obsolete: true
Comment on attachment 183429 [details] [diff] [review]
Patch updated to comments

a=brendan, with r+sr propagated.

/be
Attachment #183429 - Flags: superreview+
Attachment #183429 - Flags: review+
Attachment #183429 - Flags: approval1.8b2+
Attachment #183429 - Flags: approval-aviary1.1a1+
Blocks: 293937
Assignee: bryner → bzbarsky
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
Attachment #183357 - Flags: approval1.8b2?
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050512
Firefox/1.0+ ID:2005051214

I cannot reproduce comment #9 , browser.sessionhistory.max_viewers = 0
I cannot reproduce comment #9 , browser.sessionhistory.max_viewers =5
Now WFM with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2)
Gecko/20050512 Firefox/1.0+ ID:2005051214 regardless of what
browser.sessionhistory.max_viewers is set to.
Fix checked in... No real Tp impact, interestingly enough.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Flags: blocking1.8b2?
Depends on: 294258
Flags: blocking-aviary1.1?
Component: History: Session → Document Navigation
QA Contact: history.session → docshell
You need to log in before you can comment on or make changes to this bug.