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)

x86
Linux
defect

Tracking

()

VERIFIED FIXED

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)

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.
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?
Assignee: terry → troy
Component: Bugzilla → Layout
Product: Webtools → Browser
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.
I was using the Jan 3, 2000 daily build.
Assignee: troy → pollmann
Component: Layout → HTML Form Controls
Re-assigning to pollmann for form problem
Attaching the editparams.cgi page as a test case since access of this url
is limited.
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.
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.
Updating summary.
Summary: yet another stupid form submission bug → extra newlines added to textarea value during form submission
Keywords: dogfood, pp
Readding dogfood. Any progress on this bug?

Does not happen on Windows.
is anyone looking at  the dogfood keyword any more? I think "beta 2" would be
more appropriate if that exists yet. 
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
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
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+]
The new bug for extra trailing CR/LF's is bug 31699, thanks!
> ------- 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).

Updating QA Contact.
QA Contact: chrisd
mozilla@bucksch.org, which platform are you posting from, Win Mac or Linux?
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)
(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.
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.
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
See also bug 20207 - akkana fixed a bug today which sounds surprisingly like
this one.
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 → ---
s/You're/Your
/me goes away ashamed
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.
Adding more comments to another build with the fix removed and myself logged

out.  This is on Solaris.

Status: REOPENED → ASSIGNED
Summary: extra newlines added to textarea value during form submission → extra newlines added to hidden input during form submission
Ah ha!  Well the problem is this:

<INPUT TYPE=HIDDEN NAME="foo" VALUE="Short&#013;&#010;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.

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.
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 (&#013;) 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+]
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.
Filed bug #32000 about the bugzilla change.

Putting on the PDT- radar for beta1.
Whiteboard: [PDT-]
How is this bug related to bug 22707?
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.
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&#013;&#010;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.
Moving out to M17.
Target Milestone: M15 → M17
See bug 4928 for the background into why Bugzilla escapes newlines in the first
place. It's a long story...
Moving out to M18 - schedule update.
Target Milestone: M17 → M18
*** Bug 38826 has been marked as a duplicate of this bug. ***
QA Contact: chrisd → ckritzer
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+
Whiteboard: [PDT-] dogfood+ → [PDT-] dogfood+ fix in hand
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.
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!
Attached file reduced test case
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
Removed beta1 keyword. Perhaps this should be nominated for nsbeta2?
Keywords: beta1
Updated status whiteboard with possible landing date, based on comments above.
Whiteboard: [PDT-] dogfood+ fix in hand → [PDT-] dogfood+ fix in hand (5/31)
Eric, pollman.net is down... could you boot that bad boy back up?  Thanks dude!
-Chris
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!
Yeah. My linebreak converters should also be used in 
nsFormControlHelper::PlatformToDOMLineBreaks().
Clarifying status whiteboard (dogfood+)
Whiteboard: [PDT-] dogfood+ fix in hand (5/31) → [dogfood+] fix in hand (5/31)
Fix checked in.  I used the simple platform encoder change discussed above.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago24 years ago
Resolution: --- → FIXED
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)
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!
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.

Attachment

General

Creator:
Created:
Updated:
Size: