Closed
Bug 1490406
Opened 6 years ago
Closed 6 years ago
Nightly: Firefox doesn't respect radio groups in shadow DOM
Categories
(Core :: DOM: Core & HTML, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla64
People
(Reporter: caleb.d.williams, Assigned: smaug)
References
Details
(Keywords: nightly-community)
Attachments
(2 files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
ehsan.akhgari
:
review+
pascalc
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.81 Safari/537.36 Steps to reproduce: Testing a component that includes generated radio buttons inside a shadow root. Buttons with the same name are not counted as part of the same group and will allow you to select multiple options. There is a reproduction at https://codepen.io/calebdwilliams/pen/GXxEMv Actual results: Multiple radio buttons can be selected. Expected results: A single radio button should be selected similar to the light DOM.
Reporter | ||
Updated•6 years ago
|
Keywords: nightly-community
Updated•6 years ago
|
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bugs
Assignee | ||
Comment 2•6 years ago
|
||
remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 9 changes to 9 files (+1 heads) remote: recorded push in pushlog remote: remote: View your change here: remote: https://hg.mozilla.org/try/rev/989490ac7af27b9bbc9a243410035527b0331a79 remote: remote: Follow the progress of your build on Treeherder: remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=989490ac7af27b9bbc9a243410035527b0331a79 remote: recorded changegroup in replication log in 0.018s Not quite sure who should review this. The patch is basically just moving implementation of radio group handling from nsDocument to DocumentOrShadowRoot. nsDocument and ShadowRoot themselves still need to inherit the actual interface, since we have tons of code expecting that nsIDocument doesn't multiple inherit nsISupports. Code moves are annoying to review. Note, I'm not trying to make us to follow the spec in disconnected trees. That would be a different bug, but way too risky change for beta (I'd prefer to get this to beta). And that has nothing to do with shadow DOM.
Attachment #9010674 -
Flags: review?(ehsan)
Comment 3•6 years ago
|
||
Comment on attachment 9010674 [details] [diff] [review] shadow_radio.diff Review of attachment 9010674 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense.
Attachment #9010674 -
Flags: review?(ehsan) → review+
Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/bfb9062ec18b radio groups should work in shadow DOM, r=ehsan
Assignee | ||
Comment 5•6 years ago
|
||
Comment on attachment 9010674 [details] [diff] [review] shadow_radio.diff Approval Request Comment [Feature/Bug causing the regression]: Not a regression, but new feature, bug 1205323 [User impact if declined]: possibly broken behavior [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: just landed to m-i [Needs manual test from QE? If yes, steps to reproduce]: I don't think so [List of other uplifts needed for the feature/fix]: NA [Is the change risky?]: not too [Why is the change risky/not risky?]: Mostly just moving code upper in the class hierarchy so that it can be used also by ShadowRoot [String changes made/needed]: NA
Attachment #9010674 -
Flags: approval-mozilla-beta?
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bfb9062ec18b
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/13138 for changes under testing/web-platform/tests
Comment 8•6 years ago
|
||
Comment on attachment 9010674 [details] [diff] [review] shadow_radio.diff Fix with tests for a 63 feature, uplift approved for 63 beta 9, thanks.
Attachment #9010674 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/13138 * Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/CtHEkkUBTi6gnnL9D8EovQ)
Upstream PR merged
Comment 11•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/efa0ad8c081b
status-firefox63:
--- → fixed
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•