extra newlines added to hidden input during form submission

VERIFIED FIXED in M18

Status

()

Core
Layout: Form Controls
P3
normal
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: Dawn Endico, Assigned: Eric Pollmann)

Tracking

({pp})

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dogfood+] fix in hand (5/31); need engineer feedback (06.02), URL)

Attachments

(3 attachments)

(Reporter)

Description

18 years ago
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

18 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

18 years ago
Assignee: terry → troy
Component: Bugzilla → Layout
Product: Webtools → Browser

Comment 2

18 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

18 years ago
I was using the Jan 3, 2000 daily build.

Updated

18 years ago
Assignee: troy → pollmann
Component: Layout → HTML Form Controls

Comment 4

18 years ago
Re-assigning to pollmann for form problem
(Reporter)

Comment 5

18 years ago
Created attachment 3958 [details]
http://bugzilla.mozilla.org/editparams.cgi
(Reporter)

Comment 6

18 years ago
Attaching the editparams.cgi page as a test case since access of this url
is limited.
(Reporter)

Comment 7

18 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

18 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

18 years ago
Updating summary.
Summary: yet another stupid form submission bug → extra newlines added to textarea value during form submission

Updated

18 years ago
Keywords: dogfood, pp

Comment 10

18 years ago
Readding dogfood. Any progress on this bug?

Does not happen on Windows.
(Reporter)

Comment 11

18 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

18 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

18 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

18 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
Last Resolved: 18 years ago
Resolution: --- → FIXED
Whiteboard: [PDT+]
(Assignee)

Comment 15

18 years ago
The new bug for extra trailing CR/LF's is bug 31699, thanks!

Comment 16

18 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).

Comment 17

18 years ago
Updating QA Contact.
QA Contact: chrisd
(Assignee)

Comment 18

18 years ago
mozilla@bucksch.org, which platform are you posting from, Win Mac or Linux?

Comment 19

18 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

18 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

18 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

18 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

18 years ago
See also bug 20207 - akkana fixed a bug today which sounds surprisingly like
this one.

Comment 24

18 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

18 years ago
s/You're/Your
/me goes away ashamed
(Assignee)

Comment 26

18 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

18 years ago
(Assignee)

Comment 27

18 years ago
Adding more comments to another build with the fix removed and myself logged

out.  This is on Solaris.

(Assignee)

Updated

18 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

18 years ago
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.

Comment 29

18 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

18 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 (&#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+]

Comment 31

18 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

18 years ago
Filed bug #32000 about the bugzilla change.

Comment 33

18 years ago
Putting on the PDT- radar for beta1.
Whiteboard: [PDT-]

Comment 34

18 years ago
How is this bug related to bug 22707?
(Assignee)

Comment 35

18 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

18 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&#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.
(Assignee)

Comment 37

18 years ago
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...
(Assignee)

Comment 39

18 years ago
Moving out to M18 - schedule update.
Target Milestone: M17 → M18
(Assignee)

Comment 40

18 years ago
*** Bug 38826 has been marked as a duplicate of this bug. ***

Updated

18 years ago
QA Contact: chrisd → ckritzer
(Assignee)

Comment 41

18 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

18 years ago
Whiteboard: [PDT-] dogfood+ → [PDT-] dogfood+ fix in hand
(Assignee)

Comment 42

18 years ago
Created attachment 9363 [details] [diff] [review]
patch 1 - only convert linebreaks for textareas
(Assignee)

Comment 43

18 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

18 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

18 years ago
Created attachment 9365 [details]
reduced test case
(Assignee)

Comment 46

18 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

18 years ago
Removed beta1 keyword. Perhaps this should be nominated for nsbeta2?
Keywords: beta1

Comment 48

18 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

18 years ago
Eric, pollman.net is down... could you boot that bad boy back up?  Thanks dude!
-Chris
(Assignee)

Comment 50

18 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

18 years ago
Yeah. My linebreak converters should also be used in 
nsFormControlHelper::PlatformToDOMLineBreaks().

Comment 52

18 years ago
Clarifying status whiteboard (dogfood+)
Whiteboard: [PDT-] dogfood+ fix in hand (5/31) → [dogfood+] fix in hand (5/31)
(Assignee)

Comment 53

18 years ago
Fix checked in.  I used the simple platform encoder change discussed above.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 54

18 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

18 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

18 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.