330469 - [FIX]Submit button with empty value now has "Submit Query" text

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Martijn Wargers (dead)]

See upcoming testcase, basically this:
<input type="submit" value="">
should not give a "Submit Query" text inside the button.

This used to work in 2005-10-27 build, but not anymore in 2005-10-28 build:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-10-26+21&maxdate=2005-10-28+06&cvsroot=%2Fcvsroot

So my guess is a regression from bug 308270.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Yeah, this is a regression from the attribute landing... Martijn, for future please also cc the author of the patch that caused the regression, not just the reviewer?  ;)

Note that the code has changed some since (to use GetValue()). I'm not too happy combining that with HasAttr, but I don't really see a better way...

Comment 3

[Henrik Gemal]

dupe of 318552? or the other way since this has a patch

Comment 4

[Boris Zbarsky [:bzbarsky]]

*** Bug 318552 has been marked as a duplicate of this bug. ***

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Right fix

Comment 6

[Martijn Wargers (dead)]

(In reply to comment #2)
> Yeah, this is a regression from the attribute landing... Martijn, for future
> please also cc the author of the patch that caused the regression, not just the
> reviewer?  ;)
Uhm, my guess was  a regression from bug 308270, which you fixed, not?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, oops.  It was a regression from 311827, not 308270.  ;)  I looked at the bonsai list and didn't realize that the bug# cited in this bug was different....  It's clearly time for vacation.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 215025 [details] [diff] [review]
Right fix

Just call GetAttr instead. You're relying on that the label lives in the value-attribute anyway. (which is a pretty safe assumption i'd say)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 215025 [details] [diff] [review]
Right fix

Calling GetAttr() will get the wrong string.  The label is _based_ on the value attribute, but is not _equal_ to the value attribute.  See the blame for the code I'm modifying for details.  All I want to tell is whether what GetValue() is returning is based on something the page specified or not; if it returned null for no value attr I wouldn't have to do this, but it doesn't, so...

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Could we do this:

if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::value)) {
  elt->GetValue(aLabel);
}
else {
  rv = GetDefaultLabel(aLabel);
  NS_ENSURE_SUCCESS(rv, rv);
}

Comment 11

[Boris Zbarsky [:bzbarsky]]

We could if we want to assume GetValue is infallible.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'd be fine with that. Or you could add an NS_ENSURE_SUCCESS after GetValue. If GetValue fails I'm not sure we want to use fallback.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: With that approach

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 217181 [details] [diff] [review]
With that approach

r/sr=sicking

Comment 15

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 16

[Steuard Jensen]

Fixed on 1.8 branch by combined patch from bug 114997 comment 83 (checkin by bzbarsky, comment 86).