Using pref setter9/20/2023 ![]() ![]() > multiple times this is also used for stylesheet preloading. > copy-on-write sharing for when a document links to the same stylesheet > does is that it ensures that the style sheet's inner (which is what holds > So, I was thinking about this a little bit. I have changed the patch to ignore any ParseSelectorString errors. * IE 11 and Edge ignores all values without any console log, as they do not support setSelectorText as of writing. * Chrome ignores invalid values of selectorText without any console log. I have tested setSelectorText in Chrome, IE 11 and Edge: > be thrown from the selectorText setter? (What do other browsers do?) > So ParseSelectorString returns (throws) NS_ERROR_DOM_SYNTAX_ERR for > to look into a few of the issues that I might look into if I had a > Sorry, been very busy today, but a quick-ish review (leaving you > 0002-stylerule-selectortext-setter.patch (In reply to David Baron :dbaron: ⌚️UTC-8 from comment #29) Separate patch is good, but the first patch shouldn't land without theĪnd the tests ought to test things like invalid selectors (see above). Tests still missing, I could not find existing tests, will add as a separate patch if it is ok. What happens with a null loader in these cases? Loader is removed and nullptr is passed to CSSParser. Loader is only necessary when reporting errors(ErrorReporter.cpp mLoader) and setting quirks mode(nsCSSParser.cpp SetQuirkMode). But we still need toĪdd this assertion (*before* the WillDirty call), which means addingĪ method AssertHasUniqueInner() to CSSStyleSheet. To a StyleRule object (via DOM APIs) without this. Should be true because it shouldn't be possible for a caller to get So, really, this WillDirty call should probably be precededīy an assertion that the style sheet already has a unique inner. Unsafe to call WillDirty from a style rule, since the style rule lives inside Multiple times this is also used for stylesheet preloading. This is because style sheets useĬopy-on-write sharing for when a document links to the same stylesheet The primary thing WillDirty()ĭoes is that it ensures that the style sheet's inner (which is what holds So, I was thinking about this a little bit. But is that exception supposed toīe thrown from the selectorText setter? (What do other browsers do?) So ParseSelectorString returns (throws) NS_ERROR_DOM_SYNTAX_ERR for >+ nsresult result = css.ParseSelectorString(aSelectorText, sheet->GetSheetURI(), 0, &selectorList) To look into a few of the issues that I might look into if I had a Sorry, been very busy today, but a quick-ish review (leaving you ![]() needinfo? to xidorn for an opinion there.Ĭomment on attachment 8842541 The bigger concern is how this interacts with the Stylo work. Though it's possible there are already web-platform-tests that this will fix. ![]() Some tests (ideally web-platform-tests) are needed. (That's normally required for rule manipulation, but it's not needed if only the selector changes.) I think it's fine that this doesn't change the pointer identity of the Declaration, since nothing changes about the declaration. selectorList is an out parameter you should initialize to null, or maybe even use UniquePtr (though I don't see offhand how to use UniquePtr with out-parameters). This is wrong it will leak the object created here. >+ nsCSSSelectorList *selectorList = new nsCSSSelectorList() >+ NS_ASSERTION(loader, "Document with no CSS loader!") įor selectors, maybe the parser can be initialized with a null loader? (What things in the parser require a loader, and can selector parsing trigger any of them?) >+ // Hold strong ref to the CSSLoader in case the document update If so, can the rest of the function be made to function without the document? We should figure out whether GetDocument() being null means that no notifications need to be sent - I suspect that's the case. This seems less than optimal, since the old selector will remain. Here, and later, probably better to write out the type rather than using auto. >- // XXX then need to re-compute the cascade >- // XXX TBI - get a parser and re-parse the selectors, > StyleRule::SetSelectorText(const nsAString& aSelectorText) Though I'm not sure there's a reason to have it as a public method it's probably better to just manipulate mSelector directly in SetSelectorText. Presumably this requires a change to the header file. >+StyleRule::SetSelector(nsCSSSelectorList* aSelector) This is mostly reasonable, but a few comments: ![]()
0 Comments
Leave a Reply.AuthorWrite something about yourself. No need to be fancy, just an overview. ArchivesCategories |