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-text-3] The web de-facto requires NULL U+0000 to not be visible #6983

Closed
wants to merge 1 commit into from

Conversation

litherum
Copy link
Contributor

Chrome and Firefox make it invisible, and WebKit has compat issues about it.

https://bugs.webkit.org/show_bug.cgi?id=235559

Chrome and Firefox make it invisible, and WebKit has compat issues about it.

https://bugs.webkit.org/show_bug.cgi?id=235559
@litherum litherum changed the title [css-text-3] The web de-fact requires NULL U+0000 to not be visible [css-text-3] The web de-facto requires NULL U+0000 to not be visible Jan 25, 2022
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request Jan 25, 2022
https://bugs.webkit.org/show_bug.cgi?id=235559
<rdar://problem/87268956>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

* web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character-expected.txt: Added.
* web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character.html: Added.

Source/WebCore:

Chrome and Firefox render U+0000 NULL as invisible. We should do the same, despite it technically being classified as a control character.

w3c/csswg-drafts#6983

Test: imported/w3c/web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character.html

* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::applyCSSVisibilityRules):


Canonical link: https://commits.webkit.org/246392@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288564 268f45cc-cd09-0410-ab3c-d52691b4dbfc
annulen pushed a commit to qtwebkit/qtwebkit that referenced this pull request Jan 26, 2022
https://bugs.webkit.org/show_bug.cgi?id=235559
<rdar://problem/87268956>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

* web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character-expected.txt: Added.
* web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character.html: Added.

Source/WebCore:

Chrome and Firefox render U+0000 NULL as invisible. We should do the same, despite it technically being classified as a control character.

w3c/csswg-drafts#6983

Test: imported/w3c/web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character.html

* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::applyCSSVisibilityRules):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288564 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@frivoal frivoal added Agenda+ css-text-3 Current Work labels Jan 31, 2022
JonWBedard pushed a commit to WebKit/WebKit that referenced this pull request Feb 10, 2022
    REGRESSION(r281419): iCloud.com Notes web app fonts render incorrectly
    https://bugs.webkit.org/show_bug.cgi?id=235559
    <rdar://problem/87268956>

    Reviewed by Darin Adler.

    LayoutTests/imported/w3c:

    * web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character-expected.txt: Added.
    * web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character.html: Added.

    Source/WebCore:

    Chrome and Firefox render U+0000 NULL as invisible. We should do the same, despite it technically being classified as a control character.

    w3c/csswg-drafts#6983

    Test: imported/w3c/web-platform-tests/html/canvas/element/drawing-text-to-the-canvas/null-character.html

    * platform/graphics/WidthIterator.cpp:
    (WebCore::WidthIterator::applyCSSVisibilityRules):

    Canonical link: https://commits.webkit.org/246392@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@288564 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/245886.126@safari-613-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-613-branch@289306 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed NULL must be visible for web-compat.

The full IRC log of that discussion <TabAtkins> Topic: NULL must be visible for web-compat
<TabAtkins> github: https://github.com//pull/6983
<TabAtkins> florian: It's simple. Currently specs require every control character to be made visible. It turns out it's not web-compatible to make NULL visible. He requests we add this to the list of characters that can be invisible.
<TabAtkins> florian: Chrome and Firefox currently do that anyway.
<PeterC> What's the spec text that this proposal would impact?
<TabAtkins> florian: So to the extent that's true, I'd agree.
<TabAtkins> jfkthame_: I'd lik to see content that was affected; there doesn't appear to be examples in the webkit bug
<TabAtkins> smfr: Looks like it affected iCloud Notes
<TabAtkins> smfr: Not sure why a null character got in there
<TabAtkins> jfkthame_: It's not something we've discussed in Firefox, but I'd personally entertain making it visible in FF.
<TabAtkins> jfkthame_: Wonder if apply can't just fix iCloud if there's a problem there
<TabAtkins> jfkthame_: stray control characters in content are generally a bad thing; even worse if people can't tell they're there
<TabAtkins> astearns: That's why we had the resolution to make things visible
<TabAtkins> astearns: Has FF tried to make the other control characters visible?
<TabAtkins> jfkthame_: My recollection is we do at least on prerelease channels; unsure if we've turned it on in release. Think we've had trouble getting other browsers to line up.
<TabAtkins> astearns: Anyone from Blink have comments?
<TabAtkins> iank_: Will have to check with koji.
<TabAtkins> florian: I think there was a point in Blink where they were visible if there was a character in the font for them.
<florian> s/a character/a visible glyph/
<TabAtkins> astearns: I suggest we loop in koji or whoever else in Blink; jfkthame_ please check the state of the visible-chars code in Firefox; we'll see if there are any other compat concerns besides iCloud notes.
<TabAtkins> astearns: And we'll come back to this on a later call.
<florian> s/for them./for them, but as that's generally not the case, they were still invisible. Not sure if that got fixed./

@bfgeek
Copy link

bfgeek commented Feb 23, 2022

@kojiishi - Can you describe what our current behaviour is here, and what we've done in the past?

@astearns astearns removed the Agenda+ label Feb 23, 2022
@kojiishi
Copy link
Contributor

kojiishi commented Feb 24, 2022

Blink renders all characters in the DOM since crbug.com/530342. This caused crbug.com/550275, I have not identified why L-SEP U+2008 is visible only in Blink.

Blink doesn't do anything special to U+0000 either, but also doesn't do anything special to make sure it's visible when calling the underlying HarfBuzz or platform APIs, so it's possible that the behavior is platform and/or API dependent; i.e., if the platform API requires C-style string, it may disappear.

From a quick test, DOM and canvas measureText render U+0000 on Win10, but they don't render on mac.
https://output.jsbin.com/nuqate
I have not investigated where this difference comes from, fonts or code, yet.

@jfkthame
Copy link
Contributor

Firefox has an about:config setting layout.css.control-characters.visible that controls whether control characters are rendered visibly. Currently, this is enabled in Nightly builds, but disabled for the Beta and Release channels.

When layout.css.control-characters.visible is set to true, U+0000 is rendered as a visible "hexbox" glyph just like other control characters. I'm not aware of any reports of this being noticed in the wild (as has occasionally happened for other control codes), though perhaps shipping it more widely will unearth a few cases.

@jfkthame
Copy link
Contributor

Blink renders all characters in the DOM since crbug.com/530342.

Note that (if I'm understanding correctly) Blink simply passes the control characters through the same rendering pipeline as printable characters, so the result may be visible or not depending on the particular font being used. Some fonts map control-character codepoints to invisible glyphs, and in this case they'll remain invisible in Blink, while other fonts may render a .notdef box or even some other graphical representation.

This is not consistent with the CSS Text requirement, which says:

Control characters (Unicode category Cc)—other than tabs (U+0009), line feeds (U+000A), carriage returns (U+000D) and sequences that form a segment break—must be rendered as a visible glyph which the UA must synthesize if the glyphs found in the font are not visible

@jfkthame
Copy link
Contributor

@litherum Can you clarify what the compat concern here is about? I've tried signing in to iCloud Notes using Firefox Nightly, where the layout.css.control-characters.visible setting is true (and so any occurrences of U+0000 would appear as hexbox glyphs), but I didn't notice anything untoward. Is there a specific scenario to look at?

@litherum
Copy link
Contributor Author

litherum commented Feb 25, 2022

This came up because the Notes app on iCloud.com assumes that .notdef is invisible and has zero width. I’m assuming this one website is not the only one.

@jfkthame
Copy link
Contributor

This came up because the Notes app on iCloud.com assumes that .notdef is invisible and has zero width. I’m assuming this one website is not the only one.

But where/why does the Notes app try to render .notdef? Isn't that just a bug in the iCloud Notes app?

@kojiishi
Copy link
Contributor

kojiishi commented Mar 1, 2022

@jfkthame

Note that (if I'm understanding correctly) Blink simply passes the control characters through the same rendering pipeline as printable characters, so the result may be visible or not depending on the particular font being used. Some fonts map control-character codepoints to invisible glyphs, and in this case they'll remain invisible in Blink, while other fonts may render a .notdef box or even some other graphical representation.

Correct. Blink has not implemented that part yet, so mac users do not complain about visible L-SEP (crbug.com/550275) until we implement it.

@litherum

This came up because the Notes app on iCloud.com assumes that .notdef is invisible and has zero width. I’m assuming this one website is not the only one.

While Blink does not check U+0000, it does check glyph 0, so maybe that's what you observed in icloud.com? The glyph id 0 is .notdef as long as the font follows the Recommendations for OpenType Fonts. Blink has some places that assume all fonts follow this recommendation, such as here.

Copy link
Collaborator

@fantasai fantasai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r- until this is resolved by the WG (and also any such change needs to go into css-text-4 as well). Opened #7249 to track it as an issue.

@fantasai
Copy link
Collaborator

CSSWG rejected this change, see minutes: https://lists.w3.org/Archives/Public/www-style/2022May/0009.html
Closing out.

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 this pull request may close these issues.

8 participants