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

CSS pseudo-elements not recognized #10425

Closed
vsl7 opened this issue Aug 21, 2021 · 20 comments
Closed

CSS pseudo-elements not recognized #10425

vsl7 opened this issue Aug 21, 2021 · 20 comments
Assignees
Labels

Comments

@vsl7
Copy link

vsl7 commented Aug 21, 2021

Notepad++ doesn’t recognize CSS pseudo-elements in the form ::pseudo-element, unlike :pseudo-element. SciTE doesn’t have this problem.

@mere-human
Copy link
Contributor

Which version of SciTE did you try?

@vsl7
Copy link
Author

vsl7 commented Aug 21, 2021

@mere-human Latest version.

@mere-human
Copy link
Contributor

@mere-human Latest version.

You should post the exact numbers. So that people who'll read this in future don't need to guess what was "the latest" at that time.
Also, I'm asking because Notepad++ uses a specific version of Scintilla (4.4.6 IIRC) which may be not the latest.

@vsl7
Copy link
Author

vsl7 commented Aug 21, 2021

SciTE v. 4.4.6 also doesn’t have this problem.

@ArkadiuszMichalski
Copy link
Contributor

Only two list was set:
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/PowerEditor/src/ScintillaComponent/ScintillaEditView.h#L680

	void setCssLexer() {
		setLexer(SCLEX_CSS, L_CSS, LIST_0 | LIST_1);
	};

what represents "CSS1 Properties" and "Pseudo-classes"
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/scintilla/lexers/LexCSS.cxx#L555

Now it's wrong because pseudo-elements was listed as pseudo-classes:
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/PowerEditor/src/langs.model.xml#L122

and in practice when we use "::" for them (not ":" as for pseudoclasses) we get UNKNOWN_PSEUDOCLASS style, so even not PSEUDOCLASS style.
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/PowerEditor/src/stylers.model.xml#L331

Info:
https://developer.mozilla.org/en-US/docs/Web/CSS/Pseudo-elements

@vsl7 vsl7 changed the title CSS pseudo-elements not recognized and highlighted CSS pseudo-elements not recognized Aug 22, 2021
@vsl7
Copy link
Author

vsl7 commented Aug 22, 2021

@ArkadiuszMichalski @donho
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/PowerEditor/src/ScintillaComponent/ScintillaEditView.h#L681
Should it be
setLexer(SCLEX_CSS, L_CSS, LIST_0 | LIST_1 | LIST_0 | LIST_0 | LIST_1);
or
setLexer(SCLEX_CSS, L_CSS, LIST_0 | LIST_1 | | | LIST_1);
?

@ArkadiuszMichalski
Copy link
Contributor

I suppose LIST_0 | LIST_1 | LIST_4 (at least), but you also need to update langs.model.xml and stylers.model.xml properly.

@donho donho self-assigned this Aug 23, 2021
@donho donho added the accepted label Aug 23, 2021
@donho
Copy link
Member

donho commented Aug 26, 2021

@vsl7
Could you provide a sample of css file which contains some pseudo-elements?
Also 2 pictures of this file in SciTE and in Notepad++ please.

@vsl7
Copy link
Author

vsl7 commented Aug 26, 2021

@donho Correctly, in Notepad++ CSS pseudo-elements are recognized as UNKNOWN_PSEUDOCLASS, for which I have different highlighting.

Notepad++ v. 8.1.3 (CSS document type):
Clipboard 2

SciTE v. 4.4.6 (CSS document type):
Clipboard 1

@donho
Copy link
Member

donho commented Sep 1, 2021

@vsl7

Under my v8.1.4:

image

So it's 2 "unknown" unities are not included in the category of highlighting?

@vsl7
Copy link
Author

vsl7 commented Sep 2, 2021

@donho The problem is that known pseudo-elements in the form with double colon (::after element here) are recognized as UNKNOWN_PSEUDOCLASS. The two “unknown” elements are given for example of highlighting of unknown pseudo-elements and unknown pseudo-classes. It is seen that in Notepad++ ::after is not recognized as well as these two. In SciTE unknown elements have red highlighting, so ::after is recognized.

@donho
Copy link
Member

donho commented Sep 3, 2021

@vsl7
Considering the following CSS code you provide:

:active, ::after, :after, ::unknown, :unknown {

}

:active and :after are PSEUDOCLASS.
::after is PSEUDOELEMENT.
:unknown and ::unknown are UNKNOWN_PSEUDOCLASS & UNKNOWN_PSEUDOELEMENT respectively.

Could you confirm the description above?

@ArkadiuszMichalski
Copy link
Contributor

ArkadiuszMichalski commented Sep 3, 2021

@donho
:after is also pseudoelement (notation with : comes from old specs). In new specs only :: should be used for pseudoelement (browsers still support old notation for compatibility reasons).

In css lexer this SCE_CSS_UNKNOWN_PSEUDOCLASS represents both not recognized : and ::.

The question is what to do with the old : because the lexer doesn't recognize it as a pseudoelement (will use pseudoclass style). They can be omitted from pseudoclass words (so they will be treated as unknown, which makes sense because it's not pseudoclass anyway) or listeed in pseudoclass words and will get pseudoclass style.

I planned to open a bug for the css lexer to recognize the old : for pseudoelements, but it's only 4 cases, checking each pseudoclass for only these 4 cases can affect performance. I have encountered the same problem recently in this library:
highlightjs/highlight.js#3240

@donho
Copy link
Member

donho commented Sep 3, 2021

@ArkadiuszMichalski
Thank you for the explanation.

Could you provide me the psudo-element keyword list (or the link) please? I'm updating CSS keywords list and adding new stylers.

@donho
Copy link
Member

donho commented Sep 3, 2021

Found this link:
https://developer.mozilla.org/en-US/docs/Web/CSS/Pseudo-elements

Not sure it's all keyword list.

@ArkadiuszMichalski
Copy link
Contributor

ArkadiuszMichalski commented Sep 3, 2021

Actually MDN is official documentation for all browsers (there are also links to specs):
https://developer.mozilla.org/en-US/docs/Web/CSS/Pseudo-elements

but not much in this case, so:
https://www.w3.org/TR/CSS1/#pseudo-classes-and-pseudo-elements
https://www.w3.org/TR/CSS22/selector.html#pseudo-element-selectors
https://www.w3.org/TR/selectors-3/#pseudo-elements
https://drafts.csswg.org/selectors-4/#pseudo-elements

https://drafts.csswg.org/css-pseudo-4/
https://drafts.csswg.org/css-pseudo-4/#index-defined-here << see those with "::".

Some are also defined elsewhere, still in the testing phase. Generally, it is best to just add support for the pseudo-element list and provide some basic ones in it. This list can be completed in the future.

This are support in all current browsers:

::after (:after)
::before (:before)
::first-letter (:first-letter)
::first-line (:first-line)

::selection
::cue
::file-selector-button
::placeholder

The lexer will not recognize those with parentheses anyway (I will open issue for Lexilla later):
Lexer recognizes them, can be add:
::slotted()
::part()

@donho
Copy link
Member

donho commented Sep 3, 2021

@ArkadiuszMichalski

Thank you!

I can add "slotted part" in the list. But with or without parentheses ?

@ArkadiuszMichalski
Copy link
Contributor

ArkadiuszMichalski commented Sep 3, 2021

Without, I checked it in the last SciTE.

@ArkadiuszMichalski
Copy link
Contributor

These old pseudo-elements notation can be expressed with the "Browser-Specific Pseudo-classes" list, assuming you won't expose it anyway for it oryginal purpose:
https://github.com/notepad-plus-plus/notepad-plus-plus/blob/master/scintilla/lexers/LexCSS.cxx#L562

So use also List_6, define these 4 cases in it and use SCE_CSS_EXTENDED_PSEUDOCLASS 20 with the same style as for pseudoelements. Unfortunately, the style configurator will show EXTENDED_PSEUDOCLASS, which can be confusing (because it will cover these 4 cases and not everyone has to know about it), so it probably doesn't make sense.

donho added a commit to donho/notepad-plus-plus that referenced this issue Sep 4, 2021
Update CSS keywords and add new styles.

Fix notepad-plus-plus#10425
@donho
Copy link
Member

donho commented Sep 4, 2021

So use also List_6, define these 4 cases in it and use SCE_CSS_EXTENDED_PSEUDOCLASS 20 with the same style as for pseudoelements. Unfortunately, the style configurator will show EXTENDED_PSEUDOCLASS, which can be confusing (because it will cover these 4 cases and not everyone has to know about it), so it probably doesn't make sense.

I don't quit follow you.
Please review/comment on #10501 directly, that will be easier.

@donho donho closed this as completed in cddcbb5 Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants