Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossible to customize the style of a dialog's ::backdrop residing inside a Shadow DOM. #124

Closed
freshp86 opened this issue Mar 30, 2018 · 58 comments · Fixed by web-platform-tests/wpt#43445
Labels
topic: shadow Relates to shadow trees (as defined in DOM)

Comments

@freshp86
Copy link

Note: Copied from whatwg/html#3601. Please continue discussion here.

More context at https://bugs.chromium.org/p/chromium/issues/detail?id=827397.

Minimal repro 1

This just showcases that any CSS variable is ignored from ::backdrop elements, even without Shadow DOM usage.
https://jsfiddle.net/1zevfdce/3/

Minimal repro 2

Showcases the problem when a <dialog> resides inside a Shadow DOM.
https://jsfiddle.net/o1trLoqL/2/

Note that the spec mentions the following

"It does not inherit from any element and is not inherited from. No restrictions are made on what properties apply to this pseudo-element either."

Is that the reason for the current behavior? If yes, should the spec be revised, such that customizing the style of a ::backdrop element is possible even if it resides inside a Shadow DOM? Or is there already a viable workaround?

cc @annevk @foolip @domenic @TakayoshiKochi

@guest271314
Copy link

Have you tried without CSS var() function?

@upsuper
Copy link
Member

upsuper commented Mar 31, 2018

Is that the reason for the current behavior?

Yes. CSS variables are propagated via inheritance into descendants, so if a pseudo-element doesn't inherit from anything, CSS variables which are not defined for the pseudo-element directly would have no effect on the pseudo-element.

@guest271314
Copy link

dialog::backdrop {
  background-color: blue;
}

and


::backdrop {
  --bg-color: blue;
}

dialog::backdrop {
  background-color: var(--bg-color);
}

appear to render expected result at Chromium.

@freshp86
Copy link
Author

freshp86 commented Apr 2, 2018

@guest271314: True, that seems to work, but it still does not solve the issue of the 2nd minimal repro, which involves Shadow DOM.

@guest271314
Copy link

Have not tried Polymer. Can the code be composed without using a library?

@freshp86
Copy link
Author

freshp86 commented Apr 2, 2018

Here is a simple example that puts a <dialog> inside a Shadow root, without using Polymer. Note that I am not 100% sure that this is entirely equivalent, since it is adding a shadow root on a <div> element, instead of using a custom element.

@guest271314
Copy link

You can include <style> element within Shadow DOM to achieve the same result

let d = document.querySelector('#parent');
let shadowRoot = d.attachShadow({ mode: 'open' });
let style = document.createElement("style");
style.textContent = `::backdrop {
  --bg-color: blue;
}

dialog::backdrop {
  background-color: var(--bg-color);
}`;
let child = document.createElement('dialog');
child.textContent = 'foo';

shadowRoot.appendChild(style);
shadowRoot.appendChild(child);

let b = document.querySelector('button');
b.addEventListener('click', () => {
  child.showModal();
});

https://jsfiddle.net/062m2fhv/5/

@freshp86
Copy link
Author

freshp86 commented Apr 3, 2018

@guest271314: Thanks for your effort to help find a workaround.

Having said that, this bug is claiming that there is no proper (for example without using JS) way to style a <dialog>'s backdrop that lives within a custom element. Specifically see these docs (which are not using Polymer), on how the internals of a custom element should be able to be styled from the outside.

Your workaround shows that one can programatically create a dialog inside a Shadow DOM and, programmatically attach a style to it. It could be potentially useful, but still it seems that the current choice of the spec on how ::backdrop works, makes it impossible (I would gladly be proven wrong), to style such an element with HTML/CSS code only, such that one can stamp two instances of the custom element, and have instance A use a different backdrop than instance B.

@guest271314
Copy link

"without using JS" does not appear at OP? Not sure what you mean by "impossible", or how the linked document is related to the HTML standard? What exactly is the the requirement?

@guest271314
Copy link

such that one can stamp two instances of the custom element, and have instance A use a different backdrop than instance B

You can use the appropriate var() at CSS and select the specific <dialog> element https://jsfiddle.net/062m2fhv/7/.

@guest271314
Copy link

HTML

<button>Open blue dialog inside Shadow DOM</button>
<button>Open green dialog inside Shadow DOM</button>
<div id="parent"></div>

JavaScript

let d = document.querySelector('#parent');
let shadowRoot = d.attachShadow({
  mode: 'open'
});
let style = document.createElement("style");
style.textContent = `::backdrop {
  --bg-color: blue;
  --second-bg-color: green;
}

dialog:nth-of-type(1)::backdrop {
  background-color: var(--bg-color);
}
dialog:nth-of-type(2)::backdrop {
  background-color: var(--second-bg-color);
}
`;
let child = document.createElement('dialog');
child.textContent = 'foo';
let child1 = document.createElement('dialog');
child1.textContent = 'bar';

shadowRoot.appendChild(style);
shadowRoot.appendChild(child);
shadowRoot.appendChild(child1);

var dialogs = shadowRoot.querySelectorAll("dialog");

let b = document.querySelectorAll('button');
b.forEach((button, index) => {
  button.addEventListener('click', () => {
    dialogs[index].showModal();
  });
});

@guest271314
Copy link

Can you clarify the parameters of

with HTML/CSS code only

?

@annevk
Copy link
Member

annevk commented Apr 3, 2018

Really need @tabatkins and @fantasai's expertise to figure out what the right model for ::backdrop is I think.

And the other thing we need to know is that if we make it inherit, what would break...

@upsuper
Copy link
Member

upsuper commented Apr 3, 2018

I wonder why was it made not inherit anything? If there isn't really any compelling reason, I guess we should probably just have it inherit from the element.

@annevk
Copy link
Member

annevk commented Apr 3, 2018

There wasn't a compelling reason. As I said in the other issue it was to address https://lists.w3.org/Archives/Public/www-style/2012Jun/0441.html and back then we didn't have CSS variables. You don't think there's any observable effects to doing this? I guess most inheriting properties won't have an effect on ::backdrop.

@upsuper
Copy link
Member

upsuper commented Apr 3, 2018

There wasn't a compelling reason. As I said in the other issue it was to address https://lists.w3.org/Archives/Public/www-style/2012Jun/0441.html and back then we didn't have CSS variables.

OK. I guess that means we can make it inherit from the originating element.

You don't think there's any observable effects to doing this? I guess most inheriting properties won't have an effect on ::backdrop.

So, by default, that shouldn't affect anything. The only properties directly affect ::backdrop should be those for positioning/sizing, and border&background. I think those properties are generally not inherited.

It indeed can have observable effects if authors want to, for example, they can say background: currentcolor which would make the background depend on the inheriting property color. They can even specify explicit background: inherit. But I don't really believe that's a common case.

@upsuper
Copy link
Member

upsuper commented Apr 4, 2018

How an element/pseudo-element is rendered has no effect on where it inherits from. It's only a question of where is the element/pseudo-element logically located in the DOM tree.

e.g., ::before is assumed to be placed at the very beginning inside its originating element, so it inherits from the originating element. But you can always make it cover the whole viewport like ::backdrop via giving it some proper styles like position: fixed; top: 0; right: 0; bottom: 0; left: 0;.

In the ::backdrop case, I think the difficulty is that there is no logical location in DOM tree to place this pseudo-element. Having said that, I realized that there might be some inconsistency if we have it inherit from the originating element, because ::backdrop is not supposed to be a child of the element at all. But it's probably not a big problem, since top layer would make the pseudo-element escape from any containing block so it doesn't matter whether it is a child of anything in terms of rendering...

@guest271314
Copy link

@upsuper From the expected effect perspective the only logical element that can fathom which ::backdrop could inherit from would be <html> or :root pseudo class, though the current "inheritance" from ::backdrop appears to provide a means to utilize specificity as to selectors to display different styles for different <dialog> elements, unless missing altogether the gist of the current issue.

@freshp86
Copy link
Author

freshp86 commented Apr 4, 2018

What about dialog::backdrop? Isn't it clear in this case that the element resides within a dialog?

@guest271314
Copy link

The result of the ::backdrop affects the entire rendering of the HTML document. What is the expected result of setting properties and values at dialog::backdrop? What are you trying to achieve?

@freshp86
Copy link
Author

freshp86 commented Apr 4, 2018

Think of Web components (aka custom elements). An element that wraps a dialog is added to the DOM. The element along with its ::backdrop are rendered when showModal() is called. For example see here (which is a copy of actual code from Chromium).

As a developer, a custom element is thought of as a black box with a public API. Having to treat the ::backdrop as anything other than a child of the custom element is inconvenient, and I don't think is the suggested route anyway. FWIW dialog::backdrop is already a thing, so I don't think arguing whether the existence of it is justified as opposed to just having a global ::backdrop needs to be had in this discussion.

@guest271314
Copy link

What is the issue with the code at the link? :host::backdrop styles are applied. Still not gathering what the issue is.

@freshp86
Copy link
Author

freshp86 commented Apr 4, 2018

@guest271314: I've tried to explain the issue multiple times. See minimal repro 2 at the start of this bug.
CSS variables applied to a shadow root host don't seem to apply to a dialog::backdrop that resides under that shadow root, which is different than the behavior for any other element under the same shadow root.

@guest271314
Copy link

Is it not possible to reproduce the result at Firefox?

@freshp86
Copy link
Author

freshp86 commented Apr 4, 2018

AFAIK <dialog> is not implemented in Firefox, so no.

@tabatkins
Copy link
Contributor

tabatkins commented Apr 4, 2018

We should just say that ::backdrop inherits custom properties from its originating element, even if we explicitly block all other inheritance.

(This should be doable in pure CSS by saying that ::backdrop { all: initial; --: inherit; } applies at the UA stylesheet level; this blocks all inheritance of normal properties, but lets custom properties thru. Unfortunately I haven't defined the -- property yet. :( )

@upsuper
Copy link
Member

upsuper commented Apr 4, 2018

I don't think it makes much sense to distinguish between variables and other properties. That may add implementation complexity for no good reason.

@guest271314
Copy link

Polymer is not currently being loaded at linked jsfiddle. Can the complete issue be reproduced without using a library?

@guest271314
Copy link

A workaround which gets the style properties of host element from document.styleSheets which begin with --, and replaces any matching existing properties and values within <style> element of Shadow DOM of host element with the value defined at the style sheet

let d = document.querySelector('#parent');
let shadowRoot = d.attachShadow({
  mode: 'open'
});
let style = document.createElement("style");
let child = document.createElement('dialog');
let props = [];
for (let sheet of document.styleSheets) {
  for (let cssRules of sheet.cssRules) {
    if (document.querySelector(cssRules.selectorText) === d) {
      for (let rule of cssRules.style) {
        if (/--/.test(rule)) {
          props.push([rule, cssRules.style.getPropertyValue(rule)]);
        }
      }
    }
  }
}
style.textContent = `dialog::backdrop{background-color:var(--bg-color)}`;
shadowRoot.prepend(style);

if (props.length) {
  for (let [key, prop] of props)
    style.textContent = style.textContent.replace(`var(${key})`, prop)
}

child.textContent = 'foo';
shadowRoot.appendChild(child);

let b = document.querySelector('button');
b.addEventListener('click', () => {
  child.showModal();
});

moz-wptsync-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 2, 2023
This is as a result of a spec change (see link in the test), but it
addresses a very long-standing issue with this pseudo-element, see
whatwg/fullscreen#124.

Differential Revision: https://phabricator.services.mozilla.com/D189484

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1855668
gecko-commit: e886c398d906f9113a4ccfaba65dd10bad77f0cb
gecko-reviewers: zrhoffman
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 3, 2023
…=zrhoffman

This is as a result of a spec change (see link in the test), but it
addresses a very long-standing issue with this pseudo-element, see
whatwg/fullscreen#124.

Differential Revision: https://phabricator.services.mozilla.com/D189484
moz-wptsync-bot added a commit to web-platform-tests/wpt that referenced this issue Oct 3, 2023
This is as a result of a spec change (see link in the test), but it
addresses a very long-standing issue with this pseudo-element, see
whatwg/fullscreen#124.

Differential Revision: https://phabricator.services.mozilla.com/D189484

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1855668
gecko-commit: e886c398d906f9113a4ccfaba65dd10bad77f0cb
gecko-reviewers: zrhoffman
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Oct 3, 2023
…=zrhoffman

This is as a result of a spec change (see link in the test), but it
addresses a very long-standing issue with this pseudo-element, see
whatwg/fullscreen#124.

Differential Revision: https://phabricator.services.mozilla.com/D189484
@wes-goulet
Copy link

wes-goulet commented Oct 28, 2023

Seems like this is tracked in chromium with https://bugs.chromium.org/p/chromium/issues/detail?id=827397 (I tested in chrome with the BackdropInheritOriginating flag enabled and it worked, CSS var passed to ::backdrop).

Anyone know if there is a webkit bug for this? I couldn't find one.

Update: I created https://bugs.webkit.org/show_bug.cgi?id=263834

webkit-commit-queue pushed a commit to keithamus/WebKit that referenced this issue Nov 5, 2023
https://bugs.webkit.org/show_bug.cgi?id=263834

Reviewed by Tim Nguyen.

StyleTreeResolver/RenterTreeUpdate explicitly checked for ::backdrop
elements to remove their inheritance chain, per the spec at the time.
whatwg/fullscreen#124 was created to query
this and it was resolved that ::backdrop _should_ inherit.

This commit addresses the change by removing the check in
StyleTreeResolve to allow ::backdrop to inherit from its parent element,
and altering the check in RenderTreeUpdate to call renderer.style()
instead of renderer.view.style(). This aligns behaviour with the other
browsers and WPT.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/dialog-backdrop-create.html:
* Source/WebCore/rendering/updating/RenderTreeUpdaterGeneratedContent.cpp:
(WebCore::RenderTreeUpdater::GeneratedContent::updateBackdropRenderer):
* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::makeResolutionContextForPseudoElement):

Canonical link: https://commits.webkit.org/270246@main
@keithamus
Copy link

https://bugs.webkit.org/show_bug.cgi?id=263834 is now resolved.

Is it worth closing this issue out now?

@annevk
Copy link
Member

annevk commented Nov 6, 2023

@keithamus from that PR it seems like the only coverage for this is html/semantics/interactive-elements/the-dialog-element/backdrop-inherits.html? Which doesn't address fullscreen. I suppose it might be okay, but it doesn't seem ideal.

@keithamus
Copy link

Right of course good point.

@foolip
Copy link
Member

foolip commented Nov 30, 2023

I'm adding a test for this in web-platform-tests/wpt#43445.

Lightning00Blade pushed a commit to Lightning00Blade/wpt that referenced this issue Dec 11, 2023
This is as a result of a spec change (see link in the test), but it
addresses a very long-standing issue with this pseudo-element, see
whatwg/fullscreen#124.

Differential Revision: https://phabricator.services.mozilla.com/D189484

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1855668
gecko-commit: e886c398d906f9113a4ccfaba65dd10bad77f0cb
gecko-reviewers: zrhoffman
@yisibl
Copy link

yisibl commented Jan 9, 2024

Glad to see Chrome will be shipping soon as well: https://groups.google.com/a/chromium.org/g/blink-dev/c/yXTxBfLthzc

foolip added a commit to web-platform-tests/wpt that referenced this issue Jan 10, 2024
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 13, 2024
… ::backdrop, a=testonly

Automatic update from web-platform-tests
Test custom properties with fullscreen's ::backdrop (#43445)

Fixes whatwg/fullscreen#124.
--

wpt-commits: 6ee66f924120d02ae697a5c4c2999f27dd5ad5c0
wpt-pr: 43445
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jan 16, 2024
… ::backdrop, a=testonly

Automatic update from web-platform-tests
Test custom properties with fullscreen's ::backdrop (#43445)

Fixes whatwg/fullscreen#124.
--

wpt-commits: 6ee66f924120d02ae697a5c4c2999f27dd5ad5c0
wpt-pr: 43445

UltraBlame original commit: 5ef50e3d812feaa82c780fd762360b5045d45353
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jan 16, 2024
… ::backdrop, a=testonly

Automatic update from web-platform-tests
Test custom properties with fullscreen's ::backdrop (#43445)

Fixes whatwg/fullscreen#124.
--

wpt-commits: 6ee66f924120d02ae697a5c4c2999f27dd5ad5c0
wpt-pr: 43445

UltraBlame original commit: 5ef50e3d812feaa82c780fd762360b5045d45353
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jan 16, 2024
… ::backdrop, a=testonly

Automatic update from web-platform-tests
Test custom properties with fullscreen's ::backdrop (#43445)

Fixes whatwg/fullscreen#124.
--

wpt-commits: 6ee66f924120d02ae697a5c4c2999f27dd5ad5c0
wpt-pr: 43445

UltraBlame original commit: 5ef50e3d812feaa82c780fd762360b5045d45353
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jan 19, 2024
… ::backdrop, a=testonly

Automatic update from web-platform-tests
Test custom properties with fullscreen's ::backdrop (#43445)

Fixes whatwg/fullscreen#124.
--

wpt-commits: 6ee66f924120d02ae697a5c4c2999f27dd5ad5c0
wpt-pr: 43445
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Feb 23, 2024
github-actions bot pushed a commit to Loirooriol/stylo that referenced this issue Mar 17, 2024
…=zrhoffman

This is as a result of a spec change (see link in the test), but it
addresses a very long-standing issue with this pseudo-element, see
whatwg/fullscreen#124.

Differential Revision: https://phabricator.services.mozilla.com/D189484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: shadow Relates to shadow trees (as defined in DOM)
Development

Successfully merging a pull request may close this issue.