Closed Bug 606528 Opened 14 years ago Closed 6 years ago

The type attribute of inputs shouldn't be mapped

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(2 files, 1 obsolete file)

Don't map the type attribute for inputs
12.17 KB, patch
emilio
: review+
Details | Diff | Splinter Review
Same thing as diff -w for ease of review
11.15 KB, patch
Details | Diff | Splinter Review
We don't need to make this a mapped attribute; it just affects what the mapping function looks like. And changes to this attribute cause a reframe, so it's ok to just switch the mapping function when it changes. I wish we could also not map the various attrs image inputs need unless type=image, but we don't have a good way to change from mapped to unmapped for attrs....
Attachment #485336 - Flags: review?(dbaron)
Whiteboard: [need review]
Blocks: 473178
Comment on attachment 485336 [details] [diff] [review] Don't map the type attribute for inputs. r=dbaron if you add a comment to GetAttributeMappingFunction that you're depending on other code causing a reframe.
Attachment #485336 - Flags: review?(dbaron) → review+
Was this landed?
No, for reasons that should be obvious (not a blocker, not approved). I have no plans to land this before we ship 2.0.
Whiteboard: [need review] → [need gk2 ship]
This patch is wrong; I should have written some tests. In particular, the mapping function is baked into our mapped attrs, so we have to rebuild those when it changes....
Whiteboard: [need gk2 ship]
Attachment #485336 - Attachment is obsolete: true
Attachment #485336 - Attachment is obsolete: false
Attachment #485336 - Flags: review+ → review-
Still working on this?
Flags: needinfo?(bzbarsky)
Attachment #485336 - Attachment is obsolete: true
Flags: needinfo?(bzbarsky)
Comment on attachment 8989673 [details] [diff] [review] Don't map the type attribute for inputs Review of attachment 8989673 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsAttrAndChildArray.h @@ +128,5 @@ > > + // Update the rule mapping function on our mapped attributes, if we have any. > + // We take a nsMappedAttributeElement, not a nsMapRuleToAttributesFunc, > + // because the latter is defined in a header we can't include here. > + nsresult UpdateMappedAttrRuleMapper(nsMappedAttributeElement* aElement) nit: Maybe worth using a reference? ::: layout/reftests/forms/input/image/type-change-from-image-1-ref.html @@ +1,1 @@ > +<!DOCTYPE html> These may be worth upstreaming.
Attachment #8989673 - Flags: review?(emilio) → review+
> nit: Maybe worth using a reference? Done. > These may be worth upstreaming. Yeah, I thought about that, but I'm not sure where I put tests-to-be-upstreamed nowadays. Happy to move them somewhere. This patch makes the dom/collections/namednodemap-supported-property-names.html web platform test pass, because we no longer reorder the attributes in it. ;)
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #10) > > nit: Maybe worth using a reference? > > Done. > > > These may be worth upstreaming. > > Yeah, I thought about that, but I'm not sure where I put > tests-to-be-upstreamed nowadays. Happy to move them somewhere. layout/reftests/w3c-css/submitted/ should work, or just landing directly in testing/web-platform/tests. > This patch makes the > dom/collections/namednodemap-supported-property-names.html web platform test > pass, because we no longer reorder the attributes in it. ;) Nice!
Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7882a06d95b5 Don't map the type attribute for inputs. r=emilio
https://hg.mozilla.org/mozilla-central/rev/7882a06d95b5
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11843 for changes under testing/web-platform/tests
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: