Implement :host pseudo class #1593

Closed
opened 2 years ago by g4jc · 31 comments
g4jc commented 2 years ago (Migrated from github.com)
Owner
**Spec:** https://drafts.csswg.org/css-scoping/#selectordef-host **MDN:** https://developer.mozilla.org/en-US/docs/Web/CSS/:host() Reference Bugs: WebKit: https://bugs.webkit.org/show_bug.cgi?id=149440 Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=992245 (Rejected in favor of [Servo Only](https://bugzilla.mozilla.org/show_bug.cgi?id=1452640)) Blocks #1361
g4jc commented 2 years ago (Migrated from github.com)
Owner

Required for majority of WebComponent Demos. e.g. Fancy-Tabs.

Another Demo:
https://javascript.info/shadow-dom-style#host-selector

Required for majority of WebComponent Demos. e.g. [Fancy-Tabs](https://gist.github.com/ebidel/2d2bb0cdec3f2a16cf519dbaa791ce1b). Another Demo: https://javascript.info/shadow-dom-style#host-selector
g4jc commented 2 years ago (Migrated from github.com)
Owner

Looking at 992245, it seems a lot of this was done according to Shadow DOM v0 spec. No longer is YoungerShadow/OlderShadow applicable. I'm also unsure why this would need to touch XBL at all?

Looking at 992245, it seems a lot of this was done according to Shadow DOM v0 spec. No longer is YoungerShadow/OlderShadow applicable. I'm also unsure why this would need to touch `XBL` at all?
g4jc commented 2 years ago (Migrated from github.com)
Owner

Someone else will need to have a look at implementing this, but it does seem to me that modifications to dom/xbl/nsBindingManager.cpp are needed due to our need to "Walk the rules in shadow root" to check for CSS changes.

This is the work wchen did, minus non-working nsBindingManager.cpp changes which have been commented out. It should be a good starting point for someone to take and modify as needed and build.

0001-Implement-host-pseudo-class-host-context-CSS-selecto.txt

Someone else will need to have a look at implementing this, but it does seem to me that modifications to `dom/xbl/nsBindingManager.cpp` are needed due to our need to "Walk the rules in shadow root" to check for CSS changes. This is the work wchen did, minus non-working `nsBindingManager.cpp `changes which have been commented out. It should be a good starting point for someone to take and modify as needed and build. [0001-Implement-host-pseudo-class-host-context-CSS-selecto.txt](https://github.com/MoonchildProductions/UXP/files/4783272/0001-Implement-host-pseudo-class-host-context-CSS-selecto.txt)
athenian200 commented 2 years ago (Migrated from github.com)
Owner

I don't have much to add to what g4jc wrote, but I think it's worth noting the following:

The wchen work he's citing in that text file above isn't all from bug 992245, a good deal of it is actually from bug 1082060:

https://bugzilla.mozilla.org/show_bug.cgi?id=1082060

Mozilla themselves declined to implement host-context because Emilio objected to the way it works. wchen's work apparently would have required them to implement host and host-context in an interdependent way, which I think is part of why it was rejected.

As far as adapting wchen's old Shadow DOM v0 patch to UXP, this information might be helpful:

https://hayatoito.github.io/2016/shadowdomv1/

The important things to note are that we no longer have older or younger shadows because each shadow host can have only one shadow root now. However, we do have to account for open and closed shadow roots, which was not the case in v0. Also, not any element can be a shadow host. Finally, anything that deals with insertion points needs to be modified to account for slots instead, because slots replace insertion points in v1.

I don't have much to add to what g4jc wrote, but I think it's worth noting the following: The wchen work he's citing in that text file above isn't all from bug 992245, a good deal of it is actually from bug 1082060: https://bugzilla.mozilla.org/show_bug.cgi?id=1082060 Mozilla themselves declined to implement host-context because Emilio objected to the way it works. wchen's work apparently would have required them to implement host and host-context in an interdependent way, which I think is part of why it was rejected. As far as adapting wchen's old Shadow DOM v0 patch to UXP, this information might be helpful: https://hayatoito.github.io/2016/shadowdomv1/ The important things to note are that we no longer have older or younger shadows because each shadow host can have only one shadow root now. However, we do have to account for open and closed shadow roots, which was not the case in v0. Also, not any element can be a shadow host. Finally, anything that deals with insertion points needs to be modified to account for slots instead, because slots replace insertion points in v1.
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

@g4jc I'm not entirely sure how much of this has already been relayed through other channels but we discussed the situation (of you handing over implementation) on IRC and both @athenian200 and I are in the same boat: we both should know enough about C++ to successfully implement this from scratch (or based on wchen's work) but we don't know what we're looking at or how it all fits together.
It needs an account of the changes made so far to DOM for shadow dom and custom elements, to help us understand what was changed, how and why. Not the code side but the structural (macro) side of things.
Without that, both #1592 and this issue become impossible to implement because we don't have the information needed to make it behave as intended or connecting the right pipes of the plumbing.
Here's the logging stitched together from what I had (since #binaryoutcast on freenode logs have been wiped, apparently). https://pastebin.com/duXPaMGh

The main problem is that you @g4jc are currently the only person with the knowledge about (shadow) dom structures and customelements to continue -- and you said you can't because you run into code issues or things being Rust/Servo and can no longer port code. If you want to hand over the implementation torch for new code, then it needs a document describing what was done so far, what structures were changed (and how), and how all this new code fits in with how we handled DOM before with a singular document root and structure. Which APIs were added, which structures were created and how do they connect to the existing document structures prior to your changes? Why were these changes made, exactly? Help us understand :)

(as you yourself said the day before: if you want something implemented you have to take the time to understand all of it)

@g4jc I'm not entirely sure how much of this has already been relayed through other channels but we discussed the situation (of you handing over implementation) on IRC and both @athenian200 and I are in the same boat: we both should know enough about C++ to successfully implement this from scratch (or based on wchen's work) but we don't know what we're looking at or how it all fits together. It needs an account of the changes made so far to DOM for shadow dom and custom elements, to help us understand what was changed, how and why. Not the code side but the structural (macro) side of things. Without that, both #1592 and this issue become impossible to implement because we don't have the information needed to make it behave as intended or connecting the right pipes of the plumbing. Here's the logging stitched together from what I had (since `#binaryoutcast` on freenode logs have been wiped, apparently). https://pastebin.com/duXPaMGh The main problem is that you @g4jc are currently the only person with the knowledge about (shadow) dom structures and customelements to continue -- and you said you can't because you run into code issues or things being Rust/Servo and can no longer port code. If you want to hand over the implementation torch for new code, then it needs a document describing what was done so far, what structures were changed (and how), and how all this new code fits in with how we handled DOM before with a singular document root and structure. Which APIs were added, which structures were created and how do they connect to the existing document structures prior to your changes? Why were these changes made, exactly? Help us understand :) (as you yourself said the day before: if you want something implemented you have to take the time to understand all of it)
g4jc commented 2 years ago (Migrated from github.com)
Owner

I've updated and posted the overview of this interdependent work to the WebComponents meta #1361.

For reference, removal of Shadow DOM v0 (Multiple Roots Young/Old) was done in 5f12940329.

I've updated and [posted the overview](https://github.com/MoonchildProductions/UXP/issues/1361#issuecomment-647135193) of this interdependent work to the WebComponents meta #1361. For reference, removal of Shadow DOM v0 (Multiple Roots Young/Old) was done in https://github.com/MoonchildProductions/UXP/commit/5f12940329ba496da5730863cae94cd8c0b145da.
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

I greatly appreciate the status update (which is the thing we've actually been waiting for for quite a while yet and what Tobin has been asking for so we can let others/the public know where our development progress stands in this respect), but that isn't what I asked for here.

I greatly appreciate the status update (which is the thing we've actually been waiting for for quite a while yet and what Tobin has been asking for so we can let others/the public know where our development progress stands in this respect), but that isn't what I asked for here.
g4jc commented 2 years ago (Migrated from github.com)
Owner

Unfortunately, I do not have Doxygen or equivalent for API tracking.

Since I've never touched anything significantly in /layout, the code here should be the same as it has always been. We would know if the API had changed since all of CSS would be busted after the amount of work that has already been done in DOM. The only significant difference from existing code in DOM would have been the aforementioned removal of Shadow DOM v0's Multiple Roots since it changed the way roots work making wchen's work non-functional.

I'm willing to poke around the code for any missing APIs you think we may need, but I have literally no clue how the existing plumbing between layout/CSS and DOM works - especially in relation to XBL changes which seem to be required here.

Off-topic:
I've also added some more pseudo-selector changes we were missing to the status update post. I literally just discovered them while poking around for pseudo-element bugs. For reference, there's already a meta bug for CSS Pseudo-Elements Module Level 4 as well which I have not included there since it exceeds the scope of what we are trying to do to in order to get WebComponents working.

Unfortunately, I do not have Doxygen or equivalent for API tracking. Since I've never touched anything significantly in `/layout`, the code here should be the same as it has always been. We would know if the API had changed since all of CSS would be busted after the amount of work that has already been done in DOM. The only significant difference from existing code in DOM would have been the aforementioned removal of Shadow DOM v0's Multiple Roots since it changed the way roots work making wchen's work non-functional. I'm willing to poke around the code for any missing APIs you think we may need, but I have literally no clue how the existing plumbing between layout/CSS and DOM works - especially in relation to `XBL` changes which seem to be required here. Off-topic: I've also added some more pseudo-selector changes we were missing to the status update post. I literally just discovered them while poking around for pseudo-element bugs. For reference, there's already a meta bug for [CSS Pseudo-Elements Module Level 4](https://bugzilla.mozilla.org/show_bug.cgi?id=css-pseudo-4) as well which I have not included there since it exceeds the scope of what we are trying to do to in order to get WebComponents working.
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

I'm sorry but you don't seem to understand what's needed here?

I'll try again:
For the bugs you've ported that make structural changes, we need to know (in a descriptive explanation) in what way things have changed, and what the new structure is like/how it fits into the document/DOM/etc. structures handled by the browser. I'm assuming you understand enough of what you've ported to make a top-down/high level description of what the code changes actually do. All we have so far is bottom-level technical implementations and no overview of what it actually means.

Compare it to me telling you I exchanged Fibble for Quack. You'd understand that I swapped one thing for another, and if there is a technical description of what Fibble and Quack are made up of (API descriptions/spec reference) then you can sort-of guess how they might relate to each other or to similar things, but if you don't know that they are both part of e.g. <div> nodes, then it's impossible to know what exchanging Fibble with Quack means for the document.

Assume we know nothing about shadow DOM (which in effect we don't, since v0 was shoved in with the fork and v1 is all-new code you have implemented) or CustomElements (same story).
We need something to understand where the various APIs play a role, and how they affect the node tree and elements when used. You've taken the time to research what you've implemented (at least I sincerely hope so!); I'm asking that you condense that research and the knowledge of CE and shadow DOM you gained through that research into something we can use as a structural document to work from implementing things from scratch using both it and the spec.

I'm sorry but you don't seem to understand what's needed here? I'll try again: For the bugs you've ported that make structural changes, we need to know (in a _descriptive_ explanation) in what way things have changed, and what the new structure is like/how it fits into the document/DOM/etc. structures handled by the browser. I'm assuming you understand enough of what you've ported to make a top-down/high level description of what the code changes actually do. All we have so far is bottom-level technical implementations and no overview of what it actually means. Compare it to me telling you I exchanged Fibble for Quack. You'd understand that I swapped one thing for another, and if there is a technical description of what Fibble and Quack are made up of (API descriptions/spec reference) then you can sort-of guess how they might relate to each other or to similar things, but if you don't know that they are both part of e.g. `<div>` nodes, then it's impossible to know what exchanging Fibble with Quack means for the document. Assume we know nothing about shadow DOM (which in effect we don't, since v0 was shoved in with the fork and v1 is all-new code _you_ have implemented) or CustomElements (same story). We need something to understand where the various APIs play a role, and how they affect the node tree and elements when used. You've taken the time to research what you've implemented (at least I sincerely hope so!); I'm asking that you condense that research and the knowledge of CE and shadow DOM you gained through that research into something we can use as a structural document to work from implementing things from scratch using both it and the spec.
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

@athenian200 Maybe you can also try to explain? I feel like I'm struggling pouring this properly into words (my lack of focus not helping, I'm sure).

@athenian200 Maybe you can also try to explain? I feel like I'm struggling pouring this properly into words (my lack of focus not helping, I'm sure).
g4jc commented 2 years ago (Migrated from github.com)
Owner

Compare it to me telling you I exchanged Fibble for Quack.

The only significant change up to this point that is going to affect layout is DocumentOrShadowRoot:: instead of StyleScope::. I have a high level overview of meta bugs which is how I keep track of any of this, otherwise I feel like I don't know Quack from Fibble.

That list is available here: https://ethercalc.org/f816fs4khnxp
I can also upload an .ods or .xls if it is preferable.

> Compare it to me telling you I exchanged Fibble for Quack. The only significant change up to this point that is going to affect layout is `DocumentOrShadowRoot::` instead of `StyleScope::`. I have a high level overview of meta bugs which is how I keep track of any of this, otherwise I feel like I don't know Quack from Fibble. That list is available here: https://ethercalc.org/f816fs4khnxp I can also upload an `.ods` or `.xls` if it is preferable.
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

The only significant change up to this point that is going to affect layout is DocumentOrShadowRoot:: instead of StyleScope::.

And how is that going to help us implement ::host and ::slotted?

if you want something implemented you have to take the time to understand all of it

otherwise I feel like I don't know Quack from Fibble

So how can you say both things? You have no idea what you changed and how documents are affected by what you did?
How can you have implemented all these things that are marked implemented in your status report, without at least having a basic understanding of what you've been working on and what you have changed!?

> The only significant change up to this point that is going to affect layout is DocumentOrShadowRoot:: instead of StyleScope::. And how is that going to help us implement ::host and ::slotted? >> if you want something implemented you have to take the time to understand all of it > otherwise I feel like I don't know Quack from Fibble So how can you say both things? You have no idea what you changed and how documents are affected by what you did? How can you have implemented all these things that are marked implemented in your status report, without at least having a basic understanding of what you've been working on and what you have changed!?
athenian200 commented 2 years ago (Migrated from github.com)
Owner

Well, here's my two cents. I don't know if any of this helps, but I was looking at that document G4JC linked, and I saw something potentially relevant he hasn't touched yet...

https://bugzilla.mozilla.org/show_bug.cgi?id=1410895

This bug allows XBL documents to know about their insertion points, rather than just their insertion parents. He didn't implement this because it breaks the GUI of our applications. The reason I think it's relevant is because WebComponents is loosely based on XBL to a degree, and this bug talks about things like distributed children and parents of elements, slots, etc. It's also worth noting that something called an insertion point was used in Custom Elements v0, and slots are the v1 equivalent of insertion points.

So as far as I can tell, the plumbing needed to implement something like :host and :host-context would require similar plumbing to what's being touched in this bug for XBL for GetXBLInsertionParent/GetXBLInsertionPoint, correct? Some plumbing that knows how to get the parent or host of a Custom Element, and then we need some logic to make that work with a CSS pseudo-class.

I suspect Mozilla's early non-Servo implementation of Custom Elements depended partly on XBL patches that make it work more like WebComponents, because Shadow DOM + Custom Elements is an awful lot like XBL.

It seems like in order to implement :host, (and I know this is a lot of work) we might need to clone at least some of the stuff in layout/CSS that deals with XBL under a different name, and try to mess with it and hook it up to what we already have for Shadow DOM and Custom Elements on the DOM side of things while looking at the current spec and seeing where we fall short.

So what we need, broadly, is a Custom Elements implementation for CSS/Layout that more or less parallels what we already have for XBL, but which stays separate from XBL as much as possible to avoid busting it. The reason I'm proposing this is because the alternative appears to be just turning our XBL implementation into a Custom Elements/Shadow DOM implementation, and that's likely what Mozilla would have done if not for Stylo absorbing most of the CSS changes WebComponents required.

Well, here's my two cents. I don't know if any of this helps, but I was looking at that document G4JC linked, and I saw something potentially relevant he hasn't touched yet... https://bugzilla.mozilla.org/show_bug.cgi?id=1410895 This bug allows XBL documents to know about their insertion points, rather than just their insertion parents. He didn't implement this because it breaks the GUI of our applications. The reason I think it's relevant is because WebComponents is loosely based on XBL to a degree, and this bug talks about things like distributed children and parents of elements, slots, etc. It's also worth noting that something called an insertion point was used in Custom Elements v0, and slots are the v1 equivalent of insertion points. So as far as I can tell, the plumbing needed to implement something like :host and :host-context would require similar plumbing to what's being touched in this bug for XBL for GetXBLInsertionParent/GetXBLInsertionPoint, correct? Some plumbing that knows how to get the parent or host of a Custom Element, and then we need some logic to make that work with a CSS pseudo-class. I suspect Mozilla's early non-Servo implementation of Custom Elements depended partly on XBL patches that make it work more like WebComponents, because Shadow DOM + Custom Elements is an awful lot like XBL. It seems like in order to implement :host, (and I know this is a lot of work) we might need to clone at least some of the stuff in layout/CSS that deals with XBL under a different name, and try to mess with it and hook it up to what we already have for Shadow DOM and Custom Elements on the DOM side of things while looking at the current spec and seeing where we fall short. So what we need, broadly, is a Custom Elements implementation for CSS/Layout that more or less parallels what we already have for XBL, but which stays separate from XBL as much as possible to avoid busting it. The reason I'm proposing this is because the alternative appears to be just turning our XBL implementation _into_ a Custom Elements/Shadow DOM implementation, and that's likely what Mozilla would have done if not for Stylo absorbing most of the CSS changes WebComponents required.
athenian200 commented 2 years ago (Migrated from github.com)
Owner

Okay, so if you look at the XBL code immediately above or below what wchen's patches did, it's obvious he did, in fact, adapt pieces of what was already in XBL to process Shadow DOM stuff and got it work somehow. I've been playing around with the patch and while getting it to compile is fairly easy after accounting for a refactor or two that was done after he wrote it, I can't seem to get it to work in a way that doesn't either crash the browser or cause it to freeze when it encounters the :host selector.

The core problem is that the original patch was written to deal with multiple shadow roots, and now we don't have multiple shadow roots, but do have to worry about slots. It's really difficult to account for all that and visualize what code accounting for all that looks like.

Here are some notes I'm made as I tried to get this working...

Significant refactor in nsBindingManager since wchen (for comparing against original patch):
https://bugzilla.mozilla.org/show_bug.cgi?id=1182975

Removal of multiple shadow roots and HTMLShadowElement:
5f12940329

Addition of HTMLSlotElement:
48f602e65b

Node distribution for shadow tree slots:
e31ed5b074

Functions in nsIContent that seem potentially relevant:

 /**
   * Gets the ShadowRoot binding for this element.
   *
   * @return The ShadowRoot currently bound to this element.
   */
  inline mozilla::dom::ShadowRoot *GetShadowRoot() const;

  /**
   * Gets the root of the node tree for this content if it is in a shadow tree.
   * This method is called |GetContainingShadow| instead of |GetRootShadowRoot|
   * to avoid confusion with |GetShadowRoot|.
   *
   * @return The ShadowRoot that is the root of the node tree.
   */
  virtual mozilla::dom::ShadowRoot *GetContainingShadow() const = 0;

  /**
   * Gets the shadow host if this content is in a shadow tree. That is, the host
   * of |GetContainingShadow|, if its not null.
   *
   * @return The shadow host, if this is in shadow tree, or null.
   */
  nsIContent* GetContainingShadowHost() const;

Okay, so if you look at the XBL code immediately above or below what wchen's patches did, it's obvious he did, in fact, adapt pieces of what was already in XBL to process Shadow DOM stuff and got it work somehow. I've been playing around with the patch and while getting it to compile is fairly easy after accounting for a refactor or two that was done after he wrote it, I can't seem to get it to work in a way that doesn't either crash the browser or cause it to freeze when it encounters the :host selector. The core problem is that the original patch was written to deal with multiple shadow roots, and now we don't have multiple shadow roots, but do have to worry about slots. It's really difficult to account for all that and visualize what code accounting for all that looks like. Here are some notes I'm made as I tried to get this working... Significant refactor in nsBindingManager since wchen (for comparing against original patch): https://bugzilla.mozilla.org/show_bug.cgi?id=1182975 Removal of multiple shadow roots and HTMLShadowElement: https://github.com/MoonchildProductions/UXP/commit/5f12940329ba496da5730863cae94cd8c0b145da Addition of HTMLSlotElement: https://github.com/MoonchildProductions/UXP/commit/48f602e65b0bcb10e3a8367dbbb70185e2e33125 Node distribution for shadow tree slots: https://github.com/MoonchildProductions/UXP/commit/e31ed5b07466d4a579fe4b025f97c971003fbc3f Functions in nsIContent that seem potentially relevant: ``` /** * Gets the ShadowRoot binding for this element. * * @return The ShadowRoot currently bound to this element. */ inline mozilla::dom::ShadowRoot *GetShadowRoot() const; /** * Gets the root of the node tree for this content if it is in a shadow tree. * This method is called |GetContainingShadow| instead of |GetRootShadowRoot| * to avoid confusion with |GetShadowRoot|. * * @return The ShadowRoot that is the root of the node tree. */ virtual mozilla::dom::ShadowRoot *GetContainingShadow() const = 0; /** * Gets the shadow host if this content is in a shadow tree. That is, the host * of |GetContainingShadow|, if its not null. * * @return The shadow host, if this is in shadow tree, or null. */ nsIContent* GetContainingShadowHost() const; ```
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

So, ::host should simply be a selector for what's returned by GetContainingShadowHost(), then.

So, `::host` should simply be a selector for what's returned by `GetContainingShadowHost()`, then.
athenian200 commented 2 years ago (Migrated from github.com)
Owner

That should be the case, I think. I also found some stuff in FragmentOrElement, again right next to the XBL stuff:


   /**
     * ShadowRoot bound to the element.
     */
    RefPtr<ShadowRoot> mShadowRoot;

    /**
     * The root ShadowRoot of this element if it is in a shadow tree.
     */
    RefPtr<ShadowRoot> mContainingShadow;

    /**
     * The assigned slot associated with this element.
     */
    RefPtr<mozilla::dom::HTMLSlotElement> mAssignedSlot;

The thing about wchen is that he apparently knew a lot about XBL's implementation details. That's why he knew to go to nsBindingManager and make use of existing code the way he did. I feel like this would all be fairly easy if I understood those implementation details myself, but as things stand I'm struggling to piece all this together.

Making things worse is that there isn't much recent work done at Mozilla with respect to CSS selectors that I can refer back to, most of the actual serious implementation work is in patches are 17-20 years old from back when everything was poorly documented and scattered patches because they used CVS. They did not like touching this code at all, at least not for anything more than bugfixes, cleanup or refactoring. Not that I'm complaining, it's just hard to develop a sense of confidence that I understand what's going on with this code in light of that.

That should be the case, I think. I also found some stuff in FragmentOrElement, again right next to the XBL stuff: ``` /** * ShadowRoot bound to the element. */ RefPtr<ShadowRoot> mShadowRoot; /** * The root ShadowRoot of this element if it is in a shadow tree. */ RefPtr<ShadowRoot> mContainingShadow; /** * The assigned slot associated with this element. */ RefPtr<mozilla::dom::HTMLSlotElement> mAssignedSlot; ``` The thing about wchen is that he apparently knew a lot about XBL's implementation details. That's why he knew to go to nsBindingManager and make use of existing code the way he did. I feel like this would all be fairly easy if I understood those implementation details myself, but as things stand I'm struggling to piece all this together. Making things worse is that there isn't much recent work done at Mozilla with respect to CSS selectors that I can refer back to, most of the actual serious implementation work is in patches are 17-20 years old from back when everything was poorly documented and scattered patches because they used CVS. They did not like touching this code at all, at least not for anything more than bugfixes, cleanup or refactoring. Not that I'm complaining, it's just hard to develop a sense of confidence that I understand what's going on with this code in light of that.
wolfbeast commented 2 years ago (Migrated from github.com)
Owner

Yeah unfortunately I can't do much to help because I'm in the same boat as you are; I haven't really touched CSS selector parsing code, nor XBL.
In the meantime I've been putting work into #618 to get us completely up to spec in that respect since I'm the only one of our team who's versed enough in the innards of SpiderMonkey to work on that, and that's taking a lot of time but is necessary work for WebComponents too since it will more extensively use module type scripting in WebComponent-based frameworks.

Yeah unfortunately I can't do much to help because I'm in the same boat as you are; I haven't really touched CSS selector parsing code, nor XBL. In the meantime I've been putting work into #618 to get us completely up to spec in that respect since I'm the only one of our team who's versed enough in the innards of SpiderMonkey to work on that, and that's taking a lot of time but is necessary work for WebComponents too since it will more extensively use module type scripting in WebComponent-based frameworks.
athenian200 commented 2 years ago (Migrated from github.com)
Owner

Ah, that makes sense. I noticed you're doing a lot of good work there, and that is a load off my mind. I had a feeling that would be the next big hurdle, and it's in a completely different part of the code than the DOM/Layout stuff we've been working with. This way we won't struggle through the CSS stuff and then get blindsided by module type scripting.

Anyway, G4JC and I are starting to feel that without someone on our team who knows CSS selectors and XBL, we really only have two options here:

  1. We need to study and document this code ourselves, because Mozilla barely touched this code for years and then just ripped out everything we want to work with. This will have to be done in a way that goes beyond the scope of this issue because we'll trying to understand the broader context of how XBL and CSS selectors are handled in Goanna right now before trying to figure out how to hook in Shadow DOM v1 CSS Selectors to that existing code and make it all work together. I've tried to keep my study of the available Shadow DOM functions and such in this issue largely on-topic, but it's looking like we need to go deeper in order to get a handle on this. I don't know if there's a process in place to follow for evaluating existing code in our codebase that's not well-understood or documented already. I'm guessing we would either create an issue here, or a forum thread in Development discussion.

  2. The only other option would require us to find someone who worked with XBL and/or the pre-Stylo CSS implementation prior to 2017 to help us out, either with mentoring and explanations, or by offering to write code for us. I was able to reach out to a volunteer who was involved with the Mozilla Taiwan team (a fan of XUL) and get a response, but he was more of a coordinator than a developer, and apparently none of the developers he contacted even responded. I don't think English was his first language, but he understood well enough to try and get me in touch with the developers. I don't think anyone he contacted responded, though. It seems like e-mails he had for those people (as well as those we could find) were out of date because all those people left Mozilla soon after Stylo was implemented.

Ah, that makes sense. I noticed you're doing a lot of good work there, and that is a load off my mind. I had a feeling that would be the next big hurdle, and it's in a completely different part of the code than the DOM/Layout stuff we've been working with. This way we won't struggle through the CSS stuff and then get blindsided by module type scripting. Anyway, G4JC and I are starting to feel that without someone on our team who knows CSS selectors and XBL, we really only have two options here: 1. We need to study and document this code ourselves, because Mozilla barely touched this code for years and then just ripped out everything we want to work with. This will have to be done in a way that goes beyond the scope of this issue because we'll trying to understand the broader context of how XBL and CSS selectors are handled in Goanna right now before trying to figure out how to hook in Shadow DOM v1 CSS Selectors to that existing code and make it all work together. I've tried to keep my study of the available Shadow DOM functions and such in this issue largely on-topic, but it's looking like we need to go deeper in order to get a handle on this. I don't know if there's a process in place to follow for evaluating existing code in our codebase that's not well-understood or documented already. I'm guessing we would either create an issue here, or a forum thread in Development discussion. 2. The only other option would require us to find someone who worked with XBL and/or the pre-Stylo CSS implementation prior to 2017 to help us out, either with mentoring and explanations, or by offering to write code for us. I was able to reach out to a volunteer who was involved with the Mozilla Taiwan team (a fan of XUL) and get a response, but he was more of a coordinator than a developer, and apparently none of the developers he contacted even responded. I don't think English was his first language, but he understood well enough to try and get me in touch with the developers. I don't think anyone he contacted responded, though. It seems like e-mails he had for those people (as well as those we could find) were out of date because all those people left Mozilla soon after Stylo was implemented.
Moonchild added the
Bounty
label 2 years ago
Moonchild added a new dependency 4 months ago
athenian200 was assigned by Moonchild 4 months ago
Moonchild added the
Assigned
label 4 months ago
Owner

@athenian200 IIUC all the wchen work was for :host specifically, so I've used this issue as a vessel for adopting the work done so far by you. It came across cleanly, but I haven't tested buildability/function.

@athenian200 IIUC all the wchen work was for :host specifically, so I've used this issue as a vessel for adopting the work done so far by you. It came across cleanly, but I haven't tested buildability/function.
Collaborator

As far as I can tell, my work on this patch passes all the tests it did before and doesn't seem to crash. The problem with the menu bar items on "view source" being greyed out hasn't resurfaced either. I think a few more might even be passing than before, suddenly it seems to work a little better/faster and I'm not really sure what changed.

However, I really don't have a lot of confidence in my ability to test stuff in a thorough way. I tend to get "lucky" and not hit bugs other people do. Just something I should warn about, that's one of my big weaknesses as a programmer.

As far as I can tell, my work on this patch passes all the tests it did before and doesn't seem to crash. The problem with the menu bar items on "view source" being greyed out hasn't resurfaced either. I think a few more might even be passing than before, suddenly it seems to work a little better/faster and I'm not really sure what changed. However, I really don't have a lot of confidence in my ability to test stuff in a thorough way. I tend to get "lucky" and not hit bugs other people do. Just something I should warn about, that's one of my big weaknesses as a programmer.
Owner

suddenly it seems to work a little better/faster and I’m not really sure what changed.

unnecessary broken build system mangling is my guess :P

I really don’t have a lot of confidence in my ability to test stuff in a thorough way.

In general it means that you test known "broken" scenarios before your patch and verify it works with your patch. The other half is making sure your changes don't introduce regressions so you check if other related functionality still works as-intended.

> suddenly it seems to work a little better/faster and I’m not really sure what changed. unnecessary broken build system mangling is my guess :P > I really don’t have a lot of confidence in my ability to test stuff in a thorough way. In general it means that you test known "broken" scenarios before your patch and verify it works with your patch. The other half is making sure your changes don't introduce regressions so you check if other related functionality still works as-intended.
Owner

What would be a good test set, you think? I built with this and it seems to be fine with the pref flipped on but I don't generally go to sites that require it, and not sure what tests you have used so far. just want to give it a once-over before merging to master.

What would be a good test set, you think? I built with this and it seems to be fine with the pref flipped on but I don't generally go to sites that require it, and not sure what tests you have used so far. just want to give it a once-over before merging to master.
Collaborator

Well, all I've really done is check tests from here:

http://wpt.live/css/css-scoping

The ones labeled host and host-context.

I can't get it to work on any real sites, though, because I don't have ::slotted, and almost all real-world uses of this require that. So right now it passes tests (hot all, but most), and effectively does nothing. Because I'm very much stuck when it comes to implementing ::slotted and fixing up CSS specificity calculations, and that kind of stuff will be needed for a more serious test.

Well, all I've really done is check tests from here: http://wpt.live/css/css-scoping The ones labeled host and host-context. I can't get it to work on any real sites, though, because I don't have ::slotted, and almost all real-world uses of this require that. So right now it passes tests (hot all, but most), and effectively does nothing. Because I'm very much stuck when it comes to implementing ::slotted and fixing up CSS specificity calculations, and that kind of stuff will be needed for a more serious test.
Owner

Yeah I'm well aware of the issues with "real sites" -- it's the problem with this being an all-or-nothing specification. All of the gears will have to fit, they all have to turn, and all have to not have any teeth missing or the end result is nada.

Regardless, there do seem to be websites that are working with our half implementation as well, if i go by the forum.

This is why I was asking specifically if you had a test set you used. If that's just wpt then that's fine. I still think this is a ridiculous way of getting scoped CSS though -- just look at what we already have in <style scoped> and how easy that is to use for web designers and then compare it to this... it's utter madness. :~

Yeah I'm well aware of the issues with "real sites" -- it's the problem with this being an all-or-nothing specification. All of the gears will have to fit, they all have to turn, and all have to not have any teeth missing or the end result is nada. Regardless, there do seem to be websites that are working with our half implementation as well, if i go by the forum. This is why I was asking specifically if you had a test set you used. If that's just wpt then that's fine. I still think this is a ridiculous way of getting scoped CSS though -- just look at what we already have in `<style scoped>` and how easy that is to use for web designers and then compare it to this... it's utter madness. :~
Owner

Dropping in a quick summary:

  • host-context parsing: PASS (except :host-context(.a, .b) being allowed)
  • host-context specificity tests: PASS
  • host-descendant tests: FAIL
  • :host in DOM APIs: PASS
  • :host:host : FAIL (why should this be a thing? it's nonsense!)
  • host-nested: FAIL (style applies to nested element, when a shadow is attached to it)
    This is a really strange quirk. Attach a shadow tree, then set style on the root element, then attach a second shadow tree to the first shadow tree (child), and the style will be applied to the root element of the nested tree (i.e. == the first shadow tree)
    Perhaps this is a descendant problem, too?
    CONFLICT: F5. document vs :host with !important, important rule should win for open mode. So the spec conflicts with itself here... o.o
  • host-parsing: PASS (same exception as host-context parsing)
  • host-slotted: FAIL (no surprise, needs ::slotted)
  • host specificity tests: PASS
  • host with default namespace selector: FAIL (to fix: https://bugzil.la/1546375)
  • host invalidation when stylesheets are removed: FAIL
  • host combined with :before and :after: FAIL
Dropping in a quick summary: * host-context parsing: PASS (except `:host-context(.a, .b)` being allowed) * host-context specificity tests: PASS * host-descendant tests: FAIL * `:host` in DOM APIs: PASS * `:host:host` : FAIL (why should this be a thing? it's nonsense!) * host-nested: FAIL (style applies to nested element, when a shadow is attached to it) This is a really strange quirk. Attach a shadow tree, then set style on the root element, then attach a second shadow tree to the first shadow tree (child), and the style will be applied to the root element of the nested tree (i.e. == the first shadow tree) Perhaps this is a descendant problem, too? **CONFLICT: F5. document vs :host with !important, important rule should win for open mode.** So the spec conflicts with itself here... o.o * host-parsing: PASS (same exception as host-context parsing) * host-slotted: FAIL (no surprise, needs `::slotted`) * host specificity tests: PASS * host with default namespace selector: FAIL (to fix: https://bugzil.la/1546375) * host invalidation when stylesheets are removed: FAIL * host combined with `:before` and `:after`: FAIL
Collaborator
  • host-descendant tests: FAIL

That's another ::slotted based failure, looking at the source code. The test itself relies on that.

  • :host:host : FAIL (why should this be a thing? it's nonsense!)

Doubling up on :host selectors in this way was accepted in the standard as a way of increasing the specificity of a pseudo-class. Our code doesn't support this, so instead of just interpreting this as one host rule with greater specificity, it interprets it as two rules and can't decide which to apply because they both have the same specificity.

  • host-nested: FAIL (style applies to nested element, when a shadow is attached to it)
    This is a really strange quirk. Attach a shadow tree, then set style on the root element, then attach a second shadow tree to the first shadow tree (child), and the style will be applied to the root element of the nested tree (i.e. == the first shadow tree)
    Perhaps this is a descendant problem, too?
    CONFLICT: F5. document vs :host with !important, important rule should win for open mode. So the spec conflicts with itself here... o.o

Yeah, that one is very weird. I'm not sure how we would pass that test without breaking !important, but I'm thinking it's part of that new framework in the spec they have for weighing specificity and changing it in bizarre ways.

That is the only issue that can probably be fixed without touching anything else.

  • host invalidation when stylesheets are removed: FAIL

Actually, this is another ::slotted failure, the code for the test uses it.

  • host combined with :before and :after: FAIL

I noticed that two of the tests associated with that seem to pass, the last one does fail though. I'm not sure why, though. It's probably not going through the rules in the "right order," which is apparently very precisely defined.

So overall, the test failures really only tell us to implement ::slotted and make sure our CSS parser applies the correct weights to everything and parses in the correct order. At least that's how I'm reading it.

> * host-descendant tests: FAIL That's another `::slotted` based failure, looking at the source code. The test itself relies on that. > * `:host:host` : FAIL (why should this be a thing? it's nonsense!) Doubling up on `:host` selectors in this way was accepted in the standard as a way of increasing the specificity of a pseudo-class. Our code doesn't support this, so instead of just interpreting this as one `host` rule with greater specificity, it interprets it as two rules and can't decide which to apply because they both have the same specificity. > * host-nested: FAIL (style applies to nested element, when a shadow is attached to it) > This is a really strange quirk. Attach a shadow tree, then set style on the root element, then attach a second shadow tree to the first shadow tree (child), and the style will be applied to the root element of the nested tree (i.e. == the first shadow tree) > Perhaps this is a descendant problem, too? > **CONFLICT: F5. document vs :host with !important, important rule should win for open mode.** So the spec conflicts with itself here... o.o Yeah, that one is very weird. I'm not sure how we would pass that test without breaking `!important`, but I'm thinking it's part of that new framework in the spec they have for weighing specificity and changing it in bizarre ways. > * host with default namespace selector: FAIL (to fix: https://bugzil.la/1546375) That is the only issue that can probably be fixed without touching anything else. > * host invalidation when stylesheets are removed: FAIL Actually, this is another `::slotted` failure, the code for the test uses it. > * host combined with `:before` and `:after`: FAIL I noticed that two of the tests associated with that seem to pass, the last one does fail though. I'm not sure why, though. It's probably not going through the rules in the "right order," which is apparently very precisely defined. So overall, the test failures really only tell us to implement `::slotted` and make sure our CSS parser applies the correct weights to everything and parses in the correct order. At least that's how I'm reading it.
Owner

Tests relying on other parts of a new spec isn't a very good test. Typical Mozilla, I guess.

Anyway seems to be doing all it needs that we can test so I'll merge this in.

Tests relying on other parts of a new spec isn't a very good test. Typical Mozilla, I guess. Anyway seems to be doing all it needs that we can test so I'll merge this in.
Owner

OK, since this is bounty material... Should I go through the process of paying you for this? I noticed you recently donated to the project which, kind of makes this a strange back and forth but it's up to you :)

OK, since this is bounty material... Should I go through the process of paying you for this? I noticed you recently donated to the project which, kind of makes this a strange back and forth but it's up to you :)
Moonchild added the
Done
label 4 months ago
Collaborator

No, it's fine. I kind of saw it as more of a "bounty material if one of us doesn't do it" sort of situation, and it wasn't bounty material at the time did it technically. :) The program was not in effect while GRE was going on.

No, it's fine. I kind of saw it as more of a "bounty material if one of us doesn't do it" sort of situation, and it wasn't bounty material at the time did it technically. :) The program was not in effect while GRE was going on.
Owner

All right!
Just to clarify; I have no problem paying bounties to others no matter their relation to the project. After all, I'm the one managing all the project's funds and unless someone specifically says a donation is for a specific person involved in the project (which happened a few times) it just goes in the "Pale Moon project jar" and nobody is paid otherwise. So it doesn't really matter if it's you or travis or some new contributor, they are all treated equally.

All right! Just to clarify; I have no problem paying bounties to others no matter their relation to the project. After all, I'm the one managing all the project's funds and unless someone specifically says a donation is for a specific person involved in the project (which happened a few times) it just goes in the "Pale Moon project jar" and nobody is paid otherwise. So it doesn't really matter if it's you or travis or some new contributor, they are all treated equally.
Moonchild removed the
Bounty
label 4 months ago
Owner

Since this seems to be as complete as it's going to be right now, I'm closing this despite not being able to test ::slotted-dependent parts.

Since this seems to be as complete as it's going to be right now, I'm closing this despite not being able to test ::slotted-dependent parts.
Moonchild closed this issue 4 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Blocks
Reference: MoonchildProductions/UXP#1593
Loading…
There is no content yet.