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

All of WET 4.0's logo SVGs are implemented in a manner that fails HTML validation #8276

Closed
EricDunsworth opened this issue Feb 7, 2018 · 9 comments · Fixed by #8282
Closed

Comments

@EricDunsworth
Copy link
Member

EricDunsworth commented Feb 7, 2018

A few days ago, validator/validator#422 was resolved. One of the commits that fixed it (validator/validator@1c02bc1) intentionally caused any elements containing tabindex attributes to trigger a validation error when situated within <a>/<button> elements.

Based on my research, validator/validator@1c02bc1's behaviour is in line with the W3C's HTML spec:

So, what does this have to do with WET? It turns out that most of WET 4.0's themes use SVG site logos that are structured as links containing an <object> element with a tabindex="-1" attribute. In other words, WET is nesting interactive content (<object tabindex="-1">) within other interactive content (<a>), which goes against the HTML spec.

As a result, the following error is now appearing when validating virtually any pages that use WET:

Error: An element with the attribute tabindex must not appear as a descendant of the <a> element.

From line 61, column 1; to line 61, column 83

<object type="image/svg+xml" tabindex="-1" data="./theme-wet-boew/assets/logo.svg">

IMO the best way of fixing this would be to replace the <object> element with <img> in WET's SVG images and remove the tabindex="-1" attributes. I believe it should work fine in all modern browsers and retain compatibility with the SVG image replacer polyfill.

@duboisp @LaurentGoderre @nschonni @pjackson28 @shawnthompson Any thoughts/concerns?

PS:
The reason a tabindex="-1" attribute was used on SVG <object> elements in the first place was because of #1432 (which was fixed by wet-boew/wet-boew-legacy@cc8b81c). Without that attribute, all browsers attempt to set keyboard tabbing focus inside the <object> element - which isn't desirable.

@duboisp
Copy link
Member

duboisp commented Feb 7, 2018

@EricDunsworth great, thank for the background.

Do you know what browser required to load SVG via an object element?

@EricDunsworth
Copy link
Member Author

EricDunsworth commented Feb 7, 2018

@duboisp Based on https://stackoverflow.com/questions/4476526/do-i-use-img-object-or-embed-for-svg-files, it seems like several years ago, browser support for SVGs referenced within<img> elements was somewhat inconsistent and the <object> element was considered to be more compatible across browsers. Nowadays it sounds like the <img> element is much better off than before and is now the ideal way of referencing non-interactive SVGs.

I also stumbled upon https://developer.mozilla.org/en-US/docs/Web/SVG/SVG_as_an_Image, which states that Gecko implements several security restrictions on SVGs that are used as images (like when they're referenced via <img> elements). Those restrictions don't apply to SVGs referenced via <object> elements. But in WET's case, none of the logo images this issue covers are meant to be interactive. So I don't think the restrictions would be a show-stopper.

@duboisp
Copy link
Member

duboisp commented Feb 7, 2018

@EricDunsworth can you submit a PR and then I would create a working example for testing.

@EricDunsworth
Copy link
Member Author

@duboisp Yeah - that's my intent. Should have a PR open for this repo by COB or tomorrow depending on whether I run into any issues in my own testing.

@Bgolden67
Copy link
Member

I was about to submit this issue this morning.. thanks for this. I will await your test before i implement your suggested fix.

@LaurentGoderre
Copy link
Member

One thing to test is nested media queries in the SVG. We use one SVG that adapts on it's size to switch from the color to the black and white version.

With that being said, the SVG themselves can probably be updated to reflect more modern SVG patterns.

@EricDunsworth
Copy link
Member Author

@LaurentGoderre Thanks for the heads up about the nested media queries. Looks like support for <img> elements referencing SVGs with embedded media queries is inconsistent in modern browsers.

IE11 honours @screen correctly, but Firefox and Opera (Blink) don't. That's going to make things a lot trickier. I was thinking of maybe trying something with a filter CSS property as a workaround, but that wouldn't work in IE11. Plus it might have unexpected side effects in browsers that support filter and honour @screen correctly (possibly Edge?).

SVGs referenced via <img> elements also seem to scale slightly differently than <object> in Firefox and Opera. So I might end up modifying the CSS properties that control the logo SVG's dimensions to try exactly matching the rendered size we were getting with the <object> element.

@EricDunsworth
Copy link
Member Author

EricDunsworth commented Feb 8, 2018

I believe I've come up with a viable way of dealing with the SVGs that have nested media queries. Adding something along the lines of @media print {filter: brightness(0%);} for the logo image will force the WET SVG logo to become black when printing in Firefox/Opera, without negatively impacting IE11/Edge.

I'll send out a PR once I figure out how best way to deal with the dimensions of <img> elements that reference the logo SVGs.

EricDunsworth added a commit to EricDunsworth/wet-boew that referenced this issue Feb 9, 2018
* Fixes wet-boew#8276.
* Adds a print CSS filter to change the logo's brightness to 0%:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they inherit screen-only queries and ignore print-only queries.
  * Prevents the fallback PNG logo from being coloured white when printing in browsers that support CSS filters.
* Retains the SVG logo's inline screen media query to prevent issues in browsers that correctly handle them and lack support for CSS filters (i.e. Internet Explorer 10-11).
* Restores the splash page's "wb-sppe" class (which was inadvertently removed in wet-boew#6113):
  * Allows the splash page's logo image to be specifically targeted by the print CSS filer.
  * Indirectly brings back wet-boew#5293's fix for the splash page logo's size in Internet Explorer 10-11.
EricDunsworth added a commit to EricDunsworth/wet-boew that referenced this issue Feb 9, 2018
* Fixes wet-boew#8276.
* Adds a print CSS filter to change the logo's brightness to 0%:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
  * Prevents the fallback PNG logo from being coloured white when printing in browsers that support CSS filters.
* Retains the SVG logo's inline screen media query to prevent issues in browsers that correctly handle them and lack support for CSS filters (i.e. Internet Explorer 10-11).
* Restores the splash page's "wb-sppe" class (which was inadvertently removed in wet-boew#6113):
  * Allows the splash page's logo image to be specifically targeted by the print CSS filer.
  * Indirectly brings back wet-boew#5293's fix for the splash page logo's size in Internet Explorer 10-11 (which also fixes the fallback PNG logo's size in all browsers).
@EricDunsworth
Copy link
Member Author

Just sent out #8282.

In the end, I wasn't able to figure out how to perfectly replicate the <object> logo's rendered dimensions with <img>. But the difference is so minor that it's highly unlikely to be noticeable in practice without performing head-to-head comparisons.

EricDunsworth added a commit to EricDunsworth/wet-boew that referenced this issue Feb 14, 2018
* Fixes wet-boew#8276.
* Adds a print CSS filter to change the logo's brightness to 0%:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
  * Prevents the fallback PNG logo from being coloured white when printing in browsers that support CSS filters.
* Retains the SVG logo's inline screen media query to prevent issues in browsers that correctly handle them and lack support for CSS filters (i.e. Internet Explorer 10-11).
* Restores the splash page's "wb-sppe" class (which was inadvertently removed in wet-boew#6113):
  * Allows the splash page's logo image to be specifically targeted by the print CSS filer.
  * Indirectly brings back wet-boew#5293's fix for the splash page logo's size in Internet Explorer 10-11 (which also fixes the fallback PNG logo's size in all browsers).
EricDunsworth added a commit to EricDunsworth/GCWeb that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8287.
* Related to wet-boew/wet-boew#8276.
* Modifies SCSS to increase the bottom margins on the FIP images to replicate the empty space that was previously appearing below their object element containers.
EricDunsworth added a commit to EricDunsworth/GCWeb that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Modifies SCSS to increase the bottom margins on the FIP images to replicate the empty space that was previously appearing below their object element containers.
EricDunsworth added a commit to EricDunsworth/wet-boew that referenced this issue Feb 14, 2018
* Fixes wet-boew#8276.
* Adds a print CSS filter to change the logo's brightness to 0%:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
  * Prevents the fallback PNG logo from being coloured white when printing in browsers that support CSS filters.
* Retains the SVG logo's inline screen media query to prevent issues in browsers that correctly handle them and lack support for CSS filters (i.e. Internet Explorer 10-11).
* Restores the splash page's "wb-sppe" class (which was inadvertently removed in wet-boew#6113):
  * Allows the splash page's logo image to be specifically targeted by the print CSS filer.
  * Indirectly brings back wet-boew#5293's fix for the splash page logo's size in Internet Explorer 10-11 (which also fixes the fallback PNG logo's size in all browsers).
EricDunsworth added a commit to EricDunsworth/theme-gc-intranet that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Adds a CSS filter to change the wordmark SVG's colour to white in small view and under:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
* Modifies the wordmark SVG's inline media query to only change its colour to white in Internet Explorer 10-11:
  * Since IE10-11 correctly handle the aforementioned media queries but lack support for CSS filters, this is the most viable setup to make the wordmark render correctly in all of WET's supported browsers.
* Modifies the theme's JavaScript logic to only toggle the fallback PNG images when switching between small and under/medium and over views.
* Modifies SCSS to increase the bottom margins on the wordmark/signature images to replicate the empty space that was previously appearing below their object element containers.
EricDunsworth added a commit to EricDunsworth/theme-base that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8287.
* Related to wet-boew/wet-boew#8276.
* Removes the logo SVG's inline media query and promotes its former CSS styles be the default ones.
  * This theme's variant of the WET logo uses a black outline, so it's presentable as-is in any scenario.
EricDunsworth added a commit to EricDunsworth/theme-base that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Removes the logo SVG's inline media query and promotes its former CSS styles be the default ones.
  * This theme's variant of the WET logo uses a black outline, so it's presentable as-is in any scenario.
EricDunsworth added a commit to EricDunsworth/theme-base that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Removes the logo SVG's inline media query and promotes its former CSS styles to be the default ones.
  * This theme's variant of the WET logo uses a black outline, so it's presentable as-is in any scenario.
EricDunsworth added a commit to EricDunsworth/theme-base that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Removes the logo SVG's inline media query and promotes its former CSS styles to be the default ones.
  * This theme's variant of the WET logo uses a black outline, so it's presentable as-is in any scenario.
EricDunsworth added a commit to EricDunsworth/theme-gcwu-fegc that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Adds a CSS filter to change the FIP SVG's colour to white in screen views:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
* Modifies the signature/wordmark SVGs' inline media queries to only change their colours to white in Internet Explorer 10-11 screen views.
  * Since IE10-11 correctly handle the aforementioned media queries but lack support for CSS filters, this is the most viable setup to make the signature/wordmark render correctly in all of WET's supported browsers.
* Modifies SCSS to increase the bottom margins on the signature/wordmark images to replicate the empty space that was previously appearing below their object element containers.
EricDunsworth added a commit to EricDunsworth/theme-gcwu-fegc that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Adds a CSS filter to change the FIP SVG's colour to white in screen views:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
* Modifies the signature/wordmark SVGs' inline media queries to only change their colours to white in Internet Explorer 10-11 screen views:
  * Since IE10-11 correctly handle the aforementioned media queries but lack support for CSS filters, this is the most viable setup to make the signature/wordmark render correctly in all of WET's supported browsers.
* Modifies SCSS to increase the bottom margins on the signature/wordmark images to replicate the empty space that was previously appearing below their object element containers.
EricDunsworth added a commit to EricDunsworth/theme-gc-intranet that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Adds a CSS filter to change the wordmark SVG's colour to white in small view and under:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
* Modifies the wordmark SVG's inline media query to only change its colour to white in Internet Explorer 10-11 small view and under.
  * Since IE10-11 correctly handle the aforementioned media queries but lack support for CSS filters, this is the most viable setup to make the wordmark render correctly in all of WET's supported browsers.
* Modifies the theme's JavaScript logic to only toggle the fallback PNG images when switching between small and under/medium and over views.
* Modifies SCSS to increase the bottom margins on the signature/wordmark images to replicate the empty space that was previously appearing below their object element containers.
EricDunsworth added a commit to EricDunsworth/theme-gcwu-fegc that referenced this issue Feb 14, 2018
* Port of wet-boew/wet-boew#8282.
* Related to wet-boew/wet-boew#8276.
* Adds a CSS filter to change the signature/wordmark SVGs' colours to white in screen views:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
  * Prevents the fallback PNG logos from being coloured white when printing in browsers that support CSS filters.
* Modifies the signature/wordmark SVGs' inline media queries to only change their colours to white in Internet Explorer 10-11 screen views:
  * Since IE10-11 correctly handle the aforementioned media queries but lack support for CSS filters, this is the most viable setup to make the signature/wordmark render correctly in all of WET's supported browsers.
* Modifies SCSS to increase the bottom margins on the signature/wordmark images to replicate the empty space that was previously appearing below their object element containers.
* Prevents the signature/wordmark SVGs from disappearing in Blink and possibly Webkit's print views in certain page templates.
cmeka pushed a commit to cmeka/wet-boew that referenced this issue Aug 23, 2018
* Fixes wet-boew#8276.
* Adds a print CSS filter to change the logo's brightness to 0%:
  * Needed for Gecko, Blink and possibly Webkit due to their buggy handling of inline SVG media queries when the img element is used. When printing, they apply screen-only queries and ignore print-only queries.
  * Prevents the fallback PNG logo from being coloured white when printing in browsers that support CSS filters.
* Retains the SVG logo's inline screen media query to prevent issues in browsers that correctly handle them and lack support for CSS filters (i.e. Internet Explorer 10-11).
* Restores the splash page's "wb-sppe" class (which was inadvertently removed in wet-boew#6113):
  * Allows the splash page's logo image to be specifically targeted by the print CSS filer.
  * Indirectly brings back wet-boew#5293's fix for the splash page logo's size in Internet Explorer 10-11 (which also fixes the fallback PNG logo's size in all browsers).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants