Closed Bug 1428685 Opened 2 years ago Closed 2 years ago

Consider renaming pref dom.webcomponents.enabled to dom.webcomponents.shadowdom.enabled

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: jessica, Assigned: jessica)

References

Details

Attachments

(2 files)

Most of the Shadow DOM related code are behind "dom.webcomponents.enabled" and this pref is only used by Shadow DOM right now, so maybe we should rename it to "dom.webcomponents.shadowdom.enabled" (to match Custom Elements pref dom.webcomponents.customelements.enabled).
Yeah, that would make sense.
Priority: -- → P2
Assignee: nobody → jjong
Attached patch patch, v1.Splinter Review
(In reply to Jessica Jong [:jessica] from comment #2)
> Created attachment 8942558 [details] [diff] [review]
> patch, v1.

We need to change servo part too:
https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/servo/components/style/gecko/selector_parser.rs#335

Hi Emilio, how do you usually do this without breaking either side?
Flags: needinfo?(emilio)
So, when the patch gets reviewed, you need to land first the servo changes (doing a pull request to https://github.com/servo/servo).

As soon as that lands on autoland, you need to push the Gecko changes.

I can take care of that if you want, just let me know when the patch is ready to land :)
Flags: needinfo?(emilio)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
> So, when the patch gets reviewed, you need to land first the servo changes
> (doing a pull request to https://github.com/servo/servo).
> 
> As soon as that lands on autoland, you need to push the Gecko changes.
> 
> I can take care of that if you want, just let me know when the patch is
> ready to land :)

That'd be great, as I'm not sure how to update servo/components/style/gecko/generated/structs.rs part, I'd better see you do it once.
Let me get gecko part reviewed first.
Comment on attachment 8942858 [details]
Bug 1428685 - Use dom.webcomponents.shadowdom.enabled pref for Shadow DOM.

https://reviewboard.mozilla.org/r/213130/#review218802
Attachment #8942858 - Flags: review?(bugs) → review+
Hi Emilio,
I have created the following pull request: https://github.com/servo/servo/pull/19783
but I'm not sure whether I need to update servo/components/style/gecko/generated/structs.rs or not.
Flags: needinfo?(emilio)
Copy-pasting from IRC:

yeah, you need to update the `structs.rs` file. If you have a non-DEBUG build, you can just copy it from `objdir/dist/rust_bindings/style/`. Though given in this case it's just renaming a global I would just `sed` it, fwiw
Flags: needinfo?(emilio)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s f21c71c924168cef889e44984b2fcc7ed67c330a -d f9fe9a19adc4: rebasing 442548:f21c71c92416 "Bug 1428685 - Use dom.webcomponents.shadowdom.enabled pref for Shadow DOM. r=smaug" (tip)
merging dom/base/nsContentUtils.cpp
merging dom/base/nsContentUtils.h
merging dom/base/nsDocument.cpp
merging layout/base/crashtests/crashtests.list
merging layout/reftests/bugs/reftest.list
warning: conflicts while merging layout/base/crashtests/crashtests.list! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bdd0bea249e3
Use dom.webcomponents.shadowdom.enabled pref for Shadow DOM. r=smaug
Thanks, Emilio for updating the bindings and merging! :)
https://hg.mozilla.org/mozilla-central/rev/bdd0bea249e3
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.