ContentExtractor: remove logic for wp img lazy loading #251
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I triggered a bug on a page with a wordpress image lazy loading system based on noscript tags. In the case of this page, two noscript tags were nested for the same image. The page fetch returned two different results depending on the libxml version installed on the Debian and Alpine systems I used for the test. As far as we already have a logic to move data-lazy-* values in src and srcset, I consider that the whole logic around the noscript tag is not needed anymore. Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
Signed-off-by: Kevin Decherf <kevin@kdecherf.com>
|
See also #240 |
|
One issue is that this preserves the <p><a href="https://uxmovement.com/category/buttons/" title="Buttons" class="st-term-108 st-accent-bg">Buttons</a></p><div class="st-post-content st-entry st-clr" itemprop="text"><p>A user made the mistake of posting their private information publicly. They had accidentally set their account to public when they wanted it set to private. What inherently caused this serious error wasn’t the user. It was a bad toggle button design.</p><p>The user misperceived the toggle button that controls the public and private modes. The cue for the active state looked inactive to them. Their misperception was caused by the inverted color cues.</p><p>Inverted color cues give the active state a dark or light color, while the inactive one gets the inverse. The problem is that users can interpret either state as active due to their equal color contrast.</p><h2>Equal Color Contrast Ratios</h2><p>Color contrast indicates visual prominence, and prominence indicates an option selection. The active state should have a much higher color contrast than the inactive one. But when you use inverted color cues, it isn’t clear which button is more prominent.</p><p><img data-lazyloaded="1" src="https://uxmovement.com/wp-content/uploads/2021/01/icc-equalcontrastratio.png" class="aligncenter size-full wp-image-32206" alt="" width="510" height="222" /></p><noscript><img class="aligncenter size-full wp-image-32206" src="https://uxmovement.com/wp-content/uploads/2021/01/icc-equalcontrastratio.png" alt="" width="510" height="222" /></noscript><p>When you calculate the color contrast ratio of the toggle buttons, you get the same value. The same value means both buttons are competing for attention, resulting in a tie.</p><p>Some think the solution is to apply a color hue to the active selection. However, this would only cause users to confuse <a href="https://uxmovement.com/buttons/why-toggle-buttons-should-never-look-like-action-buttons/">toggle buttons with action buttons</a>. This confusion occurs because they’re sharing the same cues.<img data-lazyloaded="1" src="https://uxmovement.com/wp-content/uploads/2021/01/icc-cueconfusion.png" class="aligncenter size-full wp-image-32197" alt="" width="510" height="222" /></p><noscript><img class="aligncenter size-full wp-image-32197" src="https://uxmovement.com/wp-content/uploads/2021/01/icc-cueconfusion.png" alt="" width="510" height="222" /></noscript><p>They’re both buttons, but they have different affordances. A toggle button represents selected options and states. An action button performs a process or command on the system. Different affordances need different cues.</p><p>Instead of using inverted colors or color hues, the solution is to use visual depth. Depth cues use spatial contrast to help users distinguish the active state.</p><p><img data-lazyloaded="1" src="https://uxmovement.com/wp-content/uploads/2021/01/icc-cuetouse.png" class="aligncenter size-full wp-image-32198" alt="" width="600" height="150" /></p><noscript><img class="aligncenter size-full wp-image-32198" src="https://uxmovement.com/wp-content/uploads/2021/01/icc-cuetouse.png" alt="" width="600" height="150" /></noscript><p><a href="https://www.ncbi.nlm.nih.gov/pmc/articles/PMC5526975/" target="_blank" rel="noopener">Research shows</a> that elements closer in space have a significant impact on visual working memory. Users place more attention on closer elements than ones further away. Therefore, active toggles should appear closer than inactive ones. To achieve this visual effect, you have to apply surface shading and occlusion to your buttons.</p><h2>Access Full Article</h2><p>The full article goes into detail on how to apply surface shading and occlusion on different background colors. You’ll see more examples and learn design techniques to enhance your depth cues. Subscribe to access the full article.</p><p class="c1"><a class="st-theme-button" href="https://uxmovement.substack.com/subscribe?coupon=8f63b414">Access Full Article</a></p></div><section class="st-related-posts-wrap st-clr"></section> |
|
If we didn't remove that |
It depends. Browsers should respect it, thus not showing duplicates. |
|
What do you think @jtojnar ? I'm ok to keep the noscript |
|
I think it should be removed since otherwise the image might be duplicated (think webview in an app with javascript disabled for security). Could we just add a htmlawed rule removing all noscript elements at the end of processing? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I triggered a bug on a page with a wordpress image lazy loading system
based on noscript tags. In the case of this page, two noscript tags were
nested for the same image. The page fetch returned two different results
depending on the libxml version installed on the Debian and Alpine
systems I used for the test.
As far as we already have a logic to move data-lazy-* values in src and
srcset, I consider that the whole logic around the noscript tag is not
needed anymore.
The page used to trigger the bug: https://petapixel.com/2021/01/20/a-face-mask-can-double-as-a-flash-diffuser-for-better-portraits/
A HTML snippet of this page:
This change was enough to fix the issue for this page.
However I'm not fully aware of all use cases for this lazy loading/noscript mechanism so maybe more discussion is needed to ensure that we don't break anything else.