-
Notifications
You must be signed in to change notification settings - Fork 385
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
data-amp-no-unwrap
effectiveness restricted to <body>
element, disregarded in <head>
#7234
Comments
I believe this would be addressed by implementing support for |
@westonruter Thank you for opening and reopening #6603. At my end I’ve moved the I’ve vainly tried to set up a playground for the topic. |
The issue is that the style sanitizer goes through and gathers up all styles and moves them all into the one
Note that the |
@westonruter Thank you for the information! |
@westonruter Although the cited edge case prompted me to add a paragraph (“There is a small likelihood for the same to happen while JavaScript is on. On AMP pages, the related style rule is always valid, but after clicking a heading number (which is when it would come into play) it is overridden by another one, activated using the AMP framework. Reloading the page in that state would result in a lone heading as pointed in the question.”) to https://wordpress.org/plugins/anrghg/#what%20is%20the%20lone%20heading%20after%20the%20table%20of%20contents%20label%3F — it is overshadowed by a bug in scroll offset affecting AMP-HTML, reported in ampproject/amphtml#38423. On desktop, that bug is IMO an absolute deal breaker as stated in https://anrghg.sunsite.fr/publishing-helper/features/helpers/#127-amp-compatibility-is-tested-and-showcased-on-a-dedicated-site-based-on-the Both the scroll offset bug and the noscript style issue are overwhelming me just as I need to pause the plugin project while resuming my main project due since the nineties, and now in october/november 2022. |
@westonruter The scroll offset issue has already been fixed! ampproject/amphtml#35366 The only thing we need to know is that with AMP restricting support to |
@westonruter I’ve looked into It seems tricky and would require reengineering the parsing as it should register what parent the Alongside there is a new issue #7238, while in the cited use case, the present issue is rather an insignificant edge case. Probably not on other websites though. |
Bug Description
<noscript>
elements with thedata-amp-no-unwrap
property in the<head>
are unwrapped regardless.Expected Behaviour
<noscript data-amp-no-unwrap><style> /* This CSS applies only if JS is off. */ </style></noscript>
Screenshots
In the body, there is this test code:
On a page served as AMP: https://anrghg.sunsite.fr/test-amp-compat/features/helpers/#127-amp-compatibility
On a non-AMP page: https://anrghg.sunsite.fr/publishing-helper/features/helpers/#127-amp-compatibility
(before the next heading).
In the head, there is this selector on both pages:
On the non-AMP page, it is wrapped into
<noscript data-amp-no-unwrap>
.On the AMP page, it survives tree-shaking and is in the valid CSS because it is unwrapped.
The adverse effect on the AMP is that a TOC item shows up below the TOC label after clicking a heading number, then reloading the page, while JS is on.
PHP Version
8.1
Plugin Version
2.3.0
AMP plugin template mode
Standard
WordPress Version
6.0.1
Site Health
https://anrghg.sunsite.fr/test-amp-compat/wp-content/uploads/sites/3/2022/08/site-health_2022-08-30T22500200.txt
Gutenberg Version
OS(s) Affected
Linux
Browser(s) Affected
Brave
Device(s) Affected
desktop
Acceptance Criteria
No response
Implementation Brief
No response
QA Testing Instructions
No response
Demo
No response
Changelog Entry
No response
The text was updated successfully, but these errors were encountered: