Closed Bug 134911 Opened 20 years ago Closed 14 years ago

Changing browser charset encoding loses all form data

Categories

(Core :: Layout: Form Controls, defect, P2)

x86
Windows XP
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta2

People

(Reporter: tao, Assigned: smontagu)

References

(Depends on 1 open bug)

Details

(Keywords: dataloss, regression, testcase)

Attachments

(3 files)

2002-03-26-03 commercial build: 
1. use browser to load bugzilla bug report.
2. enter comments in the additional comment area (text field)
3. go to "View|Charset" to change viewing charset, e.g., from Chinese to Western.

All the text entered disappear!
*** Bug 134910 has been marked as a duplicate of this bug. ***
Reassigning to John
Assignee: rods → jkeiser
QA Contact: madhur → tpreston
nsbeta1-. We reload the document when the charset changes. A patch for this
would be risky and time consuming.
Keywords: nsbeta1nsbeta1-
This is a real Internationalization bug, and while it looks like it's related to
bug 61363, I don't think it really is.  This shouldn't require changing the way
we reload charsets; it just means we have to trigger save/restore when this
happens.  Setting dependent on bug 166636 (Move Save/Restore out of Layout)
since the triggers are going to change there.
Depends on: 166636
Priority: -- → P2
Target Milestone: --- → Future
Reproduced in 1/03/03 Trunk, Win XP. Minimised test case attached.

Keywords: testcase
Attached file Minimised testcase
Judging by bug 92473 comment 2, this is deliberate, but I don't understand why, and the testcases there are no longer available.
Assignee: john → smontagu
Duplicate of this bug: 391632
Duplicate of this bug: 151359
Summary: Changing browser viewing charset lose all form data → Changing browser charset encoding loses all form data
I just looked, and it looks to me that with the changes made in bug 108309 we can just back out the change made for bug 92473 without regressing it.  Someone should test, though.  Or at least provide a testcase for bug 92473 so I can test...
Summary: Changing browser charset encoding loses all form data → Changing browser viewing charset lose all form data
Summary: Changing browser viewing charset lose all form data → Changing browser charset encoding loses all form data
I can test that, but not before Saturday night. Meanwhile I've attached a couple of testcases to bug 92473.
Attached patch PatchSplinter Review
This backs out the change to nsDocShell.cpp from bug 92473, and indeed fixes this (and bug 336109) without regressing bug 92473. I deliberately didn't touch the changes to other files, which set the LOAD_FLAGS_CHARSET_CHANGE flag or pass it around.
Attachment #282960 - Flags: superreview?(bzbarsky)
Attachment #282960 - Flags: review?(cbiesinger)
Comment on attachment 282960 [details] [diff] [review]
Patch

sr=me, but please try to write some tests if possible?
Attachment #282960 - Flags: superreview?(bzbarsky) → superreview+
"If possible" -- ay, there's the rub. I haven't made tests for bug 382074 and friends because I couldn't work out how to simulate an encoding change in any of our test frameworks. Suggestions are welcome.
You can simulate an encoding change in the "browser" test framework by just running whatever JS runs when the user selects that menuitem...
Comment on attachment 282960 [details] [diff] [review]
Patch

Yup, tests are good :) 
This only works for <textarea>s with no initial content, as in the attached testcase: all other form elements are reset to their initial value on changing charset.
Attachment #282960 - Attachment is obsolete: true
Attachment #282960 - Flags: review?(cbiesinger)
Uh... that's really odd.  I wouldn't expect that to happen at all.  
There seems to be a separate regression that a normal reload resets form elements :(
... but not all the time. I haven't worked out exactly what are the circumstances where that happens.
Please make sure that's filed.  That should be a 1.9 blocker.  And sounds like you want to re-request review on this patch....
Flags: blocking1.9?
Comment on attachment 282960 [details] [diff] [review]
Patch

Rerequesting for now. I'm still having some trouble making it work in the browser test environment, but it works fine manually in the cases where the reload bug doesn't kick in.
Attachment #282960 - Flags: review?(cbiesinger)
Filed bug 398243 on the reset-on-reload bug.
Attached patch TestSplinter Review
Attachment #282960 - Attachment is obsolete: false
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Comment on attachment 282960 [details] [diff] [review]
Patch

r=me as well.  Ask for approval, please.  I think we should take this.
Attachment #282960 - Flags: review?(cbiesinger) → review+
Attachment #282960 - Flags: approval1.9?
Blocks: 336109
Simon, are you going to land this once the tree reopens?
Yeah, I already put it in the queue at http://wiki.mozilla.org/Firefox3/Beta2CheckinQueue#Non-Blockers
Checked in.
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Should Target Milestone be changed to Firefox 3 M10 since this will have made it in for Beta 2? :)
Target Milestone: Future → mozilla1.9 M10
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Depends on: 503681
This test is randomly failing.  See bug 503681
You need to log in before you can comment on or make changes to this bug.