Closed
Bug 22983
Opened 25 years ago
Closed 24 years ago
extra newlines added to hidden input during form submission
Categories
(Core :: Layout: Form Controls, defect, P3)
Tracking
()
VERIFIED
FIXED
M18
People
(Reporter: endico, Assigned: pollmann)
References
()
Details
(Keywords: platform-parity, Whiteboard: [dogfood+] fix in hand (5/31); need engineer feedback (06.02))
Attachments
(3 files)
24.15 KB,
text/html
|
Details | |
3.37 KB,
patch
|
Details | Diff | Splinter Review | |
173 bytes,
text/html
|
Details |
I tried to edit the bugzilla operating parameters using mozilla. I changed one field, "nummilestones", and when i hit submit, 10 others changed as well. The ones that bugzilla claimed it changed were all textareas although some textareas were left alone.
Reporter | ||
Comment 1•25 years ago
|
||
Saving new parameters This is Bugzilla: the Mozilla bug system. For more information about what Bugzilla is and what it can do, see mozilla.org's bug pages. Changed bannerhtml. Changed blurbhtml. Changed warnbannerhtml. Changed warnfooterhtml. Changed changedmail. Changed whinemail. Changed curmilestone. Changed entryheaderhtml. Changed emailregexpdesc. Changed voteremovedmail. Changed browserbugmessage. OK, done. These are the fields that bugzilla claimed to have changed. These fields seem to have the right values tho. Terry do you agree?
Updated•25 years ago
|
Assignee: terry → troy
Component: Bugzilla → Layout
Product: Webtools → Browser
Comment 2•25 years ago
|
||
It turns out that Dawn was using a recent Mozilla build to do these changes. It ended up adding newlines to textareas that had long entries in them. Seems like a bug in forms.
Reporter | ||
Comment 3•25 years ago
|
||
I was using the Jan 3, 2000 daily build.
Reporter | ||
Comment 5•25 years ago
|
||
Reporter | ||
Comment 6•25 years ago
|
||
Attaching the editparams.cgi page as a test case since access of this url is limited.
Reporter | ||
Comment 7•25 years ago
|
||
changed summary to something more useful Marked as dogfood since this makes mozilla useless for editing bugzilla parameters. The added newlines horked things overnight so that all bugzilla mail was sent out with no bug summary in the subject.
Reporter | ||
Comment 8•25 years ago
|
||
Note how each line in my last comment has an extra newline. This is no doubt the same as the initial problem which would make this bug a dup.
Assignee | ||
Comment 9•25 years ago
|
||
Updating summary.
Summary: yet another stupid form submission bug → extra newlines added to textarea value during form submission
Updated•24 years ago
|
Comment 10•24 years ago
|
||
Readding dogfood. Any progress on this bug? Does not happen on Windows.
Reporter | ||
Comment 11•24 years ago
|
||
is anyone looking at the dogfood keyword any more? I think "beta 2" would be more appropriate if that exists yet.
Assignee | ||
Comment 12•24 years ago
|
||
I think this may have been fixed with Simon's latest linefeed converter fixes, but I'll try it out to verify when my build is done.
Status: NEW → ASSIGNED
Target Milestone: M15
Assignee | ||
Comment 13•24 years ago
|
||
Assuming this comment comes out looking okay, as it does in my tests, this bug looks to be fixed. However, I noticed that the textarea is *appending* a CR/LF to the end of a string, and each time session history is used to return to a page, another CR/LF is added to the end. I'll try to trace this down as the two problems are closely related. Posting this comment with 3/15 Linux build
Comment 14•24 years ago
|
||
Putting on PDT+ radar for beta1 for the extra intermediate new line fix. Marking Resolved/Fixed - spoke with pollman, he ok'd. Please open a new bug for the trailing new lines problem.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Whiteboard: [PDT+]
Assignee | ||
Comment 15•24 years ago
|
||
The new bug for extra trailing CR/LF's is bug 31699, thanks!
Comment 16•24 years ago
|
||
> ------- Additional Comments From pollmann@netscape.com 2000-03-13 15:27 ---- > Posting this comment with 3/15 Linux build ? :-) Seriously, which fixes are you speaking about? The latest changes from sfraser I can see in bonsai are from 2000-03-09, and my build from Sun Mar 12 04:42:14 CET 2000 shows the bug (posting with this build).
Assignee | ||
Comment 18•24 years ago
|
||
mozilla@bucksch.org, which platform are you posting from, Win Mac or Linux?
Comment 19•24 years ago
|
||
Linux, RH6.0 updated with 6.1 packages. The Mozilla instance I'm using is an optimized non-debug build of my own. - Ben Bucksch (posting with 4.7 now)
Assignee | ||
Comment 20•24 years ago
|
||
(test post, please ignore) This is a really long line which I did not press Enter while typing in. Since the textarea has WRAP=HARD, it should insert one CR/LF in the middle between the words "Since" and "the" and another between "the" and "words". This is a really long line which I did press Enter while typing in. It should also have one CR/LF in the middle between "typing" and "in" and another between "and" and ""in"". Entered using 15-Mar Linux build pulled and built this morning.
Assignee | ||
Comment 21•24 years ago
|
||
I'm not having any problems with extra CR/LF's in the middle of the string, only one at the end. CC'ing Simon as maybe he can provide more info on linefeed conversion. However, I can't reproduce the bug, so I will have to leave it RESOLVED FIXED for now.
Assignee | ||
Comment 22•24 years ago
|
||
Um... Yes, today is 13-Mar not 15-Mar, the digits on the phone are too small to read or else I need my eyes checked. :) The build I am using is form 13-Mar morning
Assignee | ||
Comment 23•24 years ago
|
||
See also bug 20207 - akkana fixed a bug today which sounds surprisingly like this one.
Comment 24•24 years ago
|
||
The extra newlines only appear, if I have to login after submission. Reproduce: 1. Logout (or change you IP). Make sure, not to login before step 5. 2. Open a bug 3. Add a comment 4. Submit 5. You'll see a login page. Use that page to login. You're comment will be submitted after you did that. 6. Go back to the bug. REOPENing
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 25•24 years ago
|
||
s/You're/Your /me goes away ashamed
Assignee | ||
Comment 26•24 years ago
|
||
Adding comments with a build that had had Akkana's fix for bug 20207 removed from it. I logged out first, so extra newlines should appear.
Assignee | ||
Updated•24 years ago
|
Assignee | ||
Comment 27•24 years ago
|
||
Adding more comments to another build with the fix removed and myself logged out. This is on Solaris.
Assignee | ||
Updated•24 years ago
|
Status: REOPENED → ASSIGNED
Summary: extra newlines added to textarea value during form submission → extra newlines added to hidden input during form submission
Assignee | ||
Comment 28•24 years ago
|
||
Ah ha! Well the problem is this: <INPUT TYPE=HIDDEN NAME="foo" VALUE="Short
Line"> When this value is submitted, it will be submitted as: foo=Short%0D%0D%0ALine This causes an extra CR to be inserted. It was *really* helpful to know that you had to be logged out of bugzilla - because only in this case will your comments be cached in a hidden input before being submitted. I don't think that this is a very common scenario, and am dubious that it deserves to be PDT+. I'll look at what it takes to fix this one.
Comment 29•24 years ago
|
||
We should not be able to insert CR into the dom, even if the user/content author mistakenly says we should. It's an illegal character and the editor doesn't deal well with it. nsGfxTextControlFrame should be stripping out the CRs; it would be useful to know how the CR is being inserted, because perhaps one of the form control classes needs to be modified in a similar way.
Assignee | ||
Comment 30•24 years ago
|
||
The editor is not getting a CR in the case of this bug. The hidden input is implemented by nsGfxButtonControlFrame, which doesn't use the editor. I'm not sure that: 1) The bug is a serious one This will only cause a problem if the website intentionally puts an illegal value (
) in a form element. I've not seen any similar bugs in other websites except bugzilla. (Correct me if I'm wrong!) 2) The fix would be small If we want to make this fix consistent for all form elements, we'd have to put CR stripping code similar to that used in bug 20207 in each of the form frame classes. Since the schedule has been pushed up to the 21st, and this bug does not have a known fix (may cause regressions that won't be caught in a few days) I'd suggest moving it to beta2. Removing PDT+ and re-adding beta1 for reconsideration.
Keywords: beta1
Whiteboard: [PDT+]
Comment 31•24 years ago
|
||
I agree with all of Eric's points. We should fix bugzilla not to put the illegal character in, and we should hold off on the form element fixes 'til after beta1.
Comment 32•24 years ago
|
||
Filed bug #32000 about the bugzilla change.
Comment 34•24 years ago
|
||
How is this bug related to bug 22707?
Assignee | ||
Comment 35•24 years ago
|
||
In both of this bug and bug 20207, a CR character is being added to the content model. For bug 20207, but element involved is a textarea. For this bug it is a hidden input. The problem is that a CR is an illegal character as far as the content model is concerned. The solution that Akkana implemented for bug 20207 was to remove the CR's from the string before setting it in the content model. We should probably implement a similar fix for this bug.
Assignee | ||
Comment 36•24 years ago
|
||
Or if you're note dyslexic like me... Bug 22707 is also related to this bug, but is not the same. Bug 22707 contains something like this: <INPUT TYPE=HIDDEN NAME="foo" VALUE="Short <-----------------(CR LF here) Line"> Whereas this bug contains this: <INPUT TYPE=HIDDEN NAME="foo" VALUE="Short
Line"> Note that bug 22707 is using the actual CR and LF characters whereas in this bug we are using the HTML entities representing the CR and LF characters. The bugs are different because in the case of bug 22707, both of the characters are stripped out before being sent to the content model, but in this bug neither are. Both should be addressed separately.
Comment 38•24 years ago
|
||
See bug 4928 for the background into why Bugzilla escapes newlines in the first place. It's a long story...
Assignee | ||
Comment 40•24 years ago
|
||
*** Bug 38826 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
QA Contact: chrisd → ckritzer
Assignee | ||
Comment 41•24 years ago
|
||
See comments on bug 31699, I'm merely transfering a wrongly reported bug's dogfood+ status to the right bug for the problem.
Whiteboard: [PDT-] → [PDT-] dogfood+
Assignee | ||
Updated•24 years ago
|
Whiteboard: [PDT-] dogfood+ → [PDT-] dogfood+ fix in hand
Assignee | ||
Comment 42•24 years ago
|
||
Assignee | ||
Comment 43•24 years ago
|
||
Perhaps this fix should be in addition to not allowing CR's into the content model. The current behaviour induces platform specific behaviour into form controls that do not vary from platform to platform (hidden, checkbox, radio, buttons, submits, resets, ...) The only control I can think of that would vary from platform to platform is the textarea, which may contain platform specific linebreaks. Thus, only this control should have platform specific linebreaks converted into net linebreaks.
Assignee | ||
Comment 44•24 years ago
|
||
The only place the value of a hidden input can be set is on the content side (nsHTMLInputElement::SetValue) We'll have to filter out \r's there!
Assignee | ||
Comment 45•24 years ago
|
||
Assignee | ||
Comment 46•24 years ago
|
||
Logged out and adding this comment to test my fix on Linux. The reduced test case can also be used to verify the fix. Load the reduced test case and click Submit The Query string should be http://pollmann.net/echo.cgi?foo=bar%0D%0Abaz
Comment 47•24 years ago
|
||
Removed beta1 keyword. Perhaps this should be nominated for nsbeta2?
Keywords: beta1
Comment 48•24 years ago
|
||
Updated status whiteboard with possible landing date, based on comments above.
Whiteboard: [PDT-] dogfood+ fix in hand → [PDT-] dogfood+ fix in hand (5/31)
Comment 49•24 years ago
|
||
Eric, pollman.net is down... could you boot that bad boy back up? Thanks dude! -Chris
Assignee | ||
Comment 50•24 years ago
|
||
Simon, being quite Da Man, wrote ConvertLineBreaks to also take this param for source line breaks: eLinebreakAny I think this might be a better and more general solution. I'll come up with a test case that includes all three types of line breaks and test on all three platforms - if this works, the patch becomes a one-liner and works for many more edge cases!
Comment 51•24 years ago
|
||
Yeah. My linebreak converters should also be used in nsFormControlHelper::PlatformToDOMLineBreaks().
Comment 52•24 years ago
|
||
Clarifying status whiteboard (dogfood+)
Whiteboard: [PDT-] dogfood+ fix in hand (5/31) → [dogfood+] fix in hand (5/31)
Assignee | ||
Comment 53•24 years ago
|
||
Fix checked in. I used the simple platform encoder change discussed above.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 24 years ago
Resolution: --- → FIXED
Comment 54•24 years ago
|
||
Eric, on the reduced test case (05/30/00 19:08), should the output of the test for Content Length look like this (where '•' = disc/bullet & '°' = circle/open bullet): • foo ° bar baz If this is the correct output, then it's fixed. If not, then it's broken cross platform.
Whiteboard: [dogfood+] fix in hand (5/31) → [dogfood+] fix in hand (5/31); need engineer feedback (06.02)
Assignee | ||
Comment 55•24 years ago
|
||
Yes, that's right. Sorry, I forgot to add verify instructions. To verify (more thorough test case): 1) go to: http://blueviper/forms/hiddencrlf2.html 2) Click Submit Query 3) The URL that is submitted to should be: http://pollmann.net/echo.cgi?foo=bar%0D%0Abaz&bar=baz%0D%0Afoo&baz=foo%0D0Abar This means that no matter what type of line feeds are added to an input, they are always converted to net line feeds (%0D%0A) when they are submitted. All three values (foo, bar, and baz) should have a line break in the middle of their value on the resulting page, like you observed before - single line break with no double-spacing! • foo ° bar baz • bar ° baz foo • baz ° foo bar Thanks!
Comment 56•24 years ago
|
||
Good testcase Eric...thanks! Marking VERIFED FIXED on: - MacOS9 2000-06-05-08-M16 Commercial Build - Linux6 2000-06-05-08-M16 Commercial Build - Win98 2000-06-05-08-M16 Commercial Build
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•