Remove Unicode case-insensitive matching for radio button groups and usemap attributes

RESOLVED FIXED in Firefox 53

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
a year ago
8 months ago

People

(Reporter: Domenic Denicola, Assigned: jdai)

Tracking

(Depends on: 1 bug)

Trunk
mozilla53
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20161019084923

Steps to reproduce:

In https://github.com/whatwg/html/commit/6acdb2122298d2bb7bb839c0a61b4e1f9b0f9bc9 we changed the HTML spec to remove all of its instances of "compatibility caseless" Unicode case-insensitive matching. There were three:

- <object usemap> matching <map name>
- <img usemap> matching <map name>
- radio button group names

All of these have been replaced with simple case-sensitive matching.

We believe this change will be low-risk due to Chromium use counter data from the stable channel, which reveals that <0.0001% of pages match using ASCII case-insensitive matching, but not using case-sensitive mapping. Furthermore, zero additional matches are triggered by Chrome's Unicode case-insensitive matching. Another reason to believe this is low-risk is that the particular Unicode case-insensitive matching used was highly inconsistent and unreliable across browsers; the spec's "compatibility caseless" was fiction.

More background at https://github.com/whatwg/html/issues/1666 and links contained therein. Web platform tests available at:

- http://w3c-test.org/html/semantics/forms/the-input-element/radio-groupname-case.html
- http://w3c-test.org/html/semantics/embedded-content/the-img-element/usemap-casing.html
- http://w3c-test.org/html/semantics/embedded-content/the-object-element/usemap-casing.html

However for the latter two Firefox will not be able to pass them until https://bugzilla.mozilla.org/show_bug.cgi?id=1227469 is fixed.

Note: for radio button groups, this is requesting a revert of https://bugzilla.mozilla.org/show_bug.cgi?id=177269
John, are you interested in taking a look?
Flags: needinfo?(jdai)
(Assignee)

Comment 2

a year ago
Yeah, I will take a look. (Keeping NI for tracking)
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

a year ago
Assignee: nobody → jdai
Flags: needinfo?(jdai)
(Assignee)

Comment 3

11 months ago
Created attachment 8823162 [details] [diff] [review]
Bug 1312456 - Part 1: Remove Unicode case-insensitive matching for radio button groups.

In this patch, I revised radio button groups to Unicode case-sensitive matching and updated html/semantics/forms/the-input-element/radio.html tests to align w3c/web-platform-tests.
(Assignee)

Comment 4

11 months ago
Created attachment 8823500 [details] [diff] [review]
Bug 1312456 - Part 2: Remove Unicode case-insensitive matching for usemap attributes.

Try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dd20c5f0cf19c6bc2fd289389ac461122dc225f5&filter-tier=1&group_state=expanded
Attachment #8823500 - Flags: review?(bugs)
(Assignee)

Updated

11 months ago
Attachment #8823162 - Flags: review?(bugs)
Comment on attachment 8823162 [details] [diff] [review]
Bug 1312456 - Part 1: Remove Unicode case-insensitive matching for radio button groups.

>+  }, "radio inputs with non-ASCII name attributes belong to the same radio button group");
perhaps s/with non/with the same non/


Are there tests for the case when group name matches only case insensitively? If not, please add some.


>   }, "changing the name of a radio input element and setting its checkedness to true makes all the other elements' checkedness in the same radio button group be set to false");
>-
Please don't make this kind of unrelated changes. Here and elsewhere in this file
Attachment #8823162 - Flags: review?(bugs) → review+
Comment on attachment 8823500 [details] [diff] [review]
Bug 1312456 - Part 2: Remove Unicode case-insensitive matching for usemap attributes.

Why can't you use AttrValueIs with the ::name too here?
That would avoid extra copying.

And can we have some (wpt) test for this?

(I wonder why we support also id attribute there, but that isn't about this bug.)
Attachment #8823500 - Flags: review?(bugs) → review-
( I filed bug 1328671 about the id attribute )
(Assignee)

Comment 8

11 months ago
(In reply to Olli Pettay [:smaug] from comment #5)
> Comment on attachment 8823162 [details] [diff] [review]
> Bug 1312456 - Part 1: Remove Unicode case-insensitive matching for radio
> button groups.
> 
> >+  }, "radio inputs with non-ASCII name attributes belong to the same radio button group");
> perhaps s/with non/with the same non/
> 
> 
> Are there tests for the case when group name matches only case
> insensitively? If not, please add some.
Yes. In comment #0 mention, we have WPT test available at:
- http://w3c-test.org/html/semantics/forms/the-input-element/radio-groupname-case.html

> 
> 
> >   }, "changing the name of a radio input element and setting its checkedness to true makes all the other elements' checkedness in the same radio button group be set to false");
> >-
> Please don't make this kind of unrelated changes. Here and elsewhere in this
> file

Will do.
(Assignee)

Comment 9

11 months ago
(In reply to Olli Pettay [:smaug] from comment #6)
> Comment on attachment 8823500 [details] [diff] [review]
> Bug 1312456 - Part 2: Remove Unicode case-insensitive matching for usemap
> attributes.
> 
> Why can't you use AttrValueIs with the ::name too here?
> That would avoid extra copying.
Will do.

> 
> And can we have some (wpt) test for this?
In comment #0 mention, we have WPT tests available at:
- http://w3c-test.org/html/semantics/embedded-content/the-img-element/usemap-casing.html
- http://w3c-test.org/html/semantics/embedded-content/the-object-element/usemap-casing.html
> 
> (I wonder why we support also id attribute there, but that isn't about this
> bug.)
(Assignee)

Comment 10

11 months ago
Created attachment 8823968 [details] [diff] [review]
Bug 1312456 - Part 1: Remove Unicode case-insensitive matching for radio button groups. r=smaug

Addressed comment #8 and carried r+.
Attachment #8823162 - Attachment is obsolete: true
Attachment #8823968 - Flags: review+
(Assignee)

Comment 11

11 months ago
Created attachment 8823970 [details] [diff] [review]
Bug 1312456 - Part 2: Remove Unicode case-insensitive matching for usemap attributes. r=smaug

Addressed comment #6.

Try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=64643e0f1d890e95ea757b042bdde388628794c8&filter-tier=1&group_state=expanded
Attachment #8823500 - Attachment is obsolete: true
Attachment #8823970 - Flags: review?(bugs)
Attachment #8823970 - Flags: review?(bugs) → review+
(Assignee)

Updated

11 months ago
Attachment #8823970 - Attachment description: Bug 1312456 - Part 2: Remove Unicode case-insensitive matching for usemap attributes. v2 → Bug 1312456 - Part 2: Remove Unicode case-insensitive matching for usemap attributes. r=smaug
(Assignee)

Updated

11 months ago
Keywords: checkin-needed

Comment 12

11 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b6b4e4825a4
Part 1: Remove Unicode case-insensitive matching for radio button groups. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/228e38354443
Part 2: Remove Unicode case-insensitive matching for usemap attributes. r=smaug
Keywords: checkin-needed

Comment 13

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1b6b4e4825a4
https://hg.mozilla.org/mozilla-central/rev/228e38354443
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox53: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla53

Updated

8 months ago
Depends on: 1358967
You need to log in before you can comment on or make changes to this bug.