[4.0] modify #31190

[N6REJ]

Pull Request for Issue # .

Summary of Changes

addresses #31516
removes api and replaces it with overrideable call

Testing Instructions

apply pr
run npm ci
check logins via https and verify logo shows.

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

remove documentation documenting htmlhelper::icon.svg

[sandewt]

I have tested this item 🔴 unsuccessfully on e2bc96c

Warning message: the path is not correct.

Warning: file_get_contents(C:\xampp\htdocs\bugtesting2\joomla/bugtesting2/joomla/media/plg_system_webauthn/images/webauthn.svg): ...

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.}

[sandewt]

fill="currentColor" is not defined

[N6REJ]

That allows css to handle the color

[sandewt]

Changing the color in eg fill="#ff0000" has no effect.

[N6REJ]

are you applying the fill to the svg or the path? It makes a difference.

[sandewt]

When I change the current color in the code, for example to red, the icon color does not change in the browser. This remains white.

[N6REJ]

correct. The only way to change the color via css is to set to "currentColor" in the svg. And then style it in the css. This is what is occuring...

[sandewt]

Try something like this:

<?php echo file_get_contents(JPATH_ROOT . substr(HTMLHelper::_('image', $button['image'], '', '', true, true), strlen(Uri::root(true)))); ?>

Build in a condition if the .svg does not exist.

[N6REJ]

@sandewt let me know if that solves your subfolder issue please.

[ceford]

I have tested this item ✅ successfully on e6e9b8d

Needs better testing instructions! I enabled Two-Factor Authentication but forgot I needed https to test. With that done I see the logo on both login locations but I get an extra box to enter Secret Key. I guess that is normal.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.}

[ghost]

I have tested this item ✅ successfully on e6e9b8d

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.}

[sandewt]

Build in a condition if the .svg does not exist.

I do this kind of check at the Improvement Vote Plugin #31098, to avoid a warning message.

// Get the image
$image = HTMLHelper::_('image', 'plg_system_webauthn/webauthn.svg', '', '', true, true);

// If you can't find the image then skip it
if ($image === null)
{
	return;
}

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.}

[sandewt]

The aria-hidden property tells screen-readers if they should ignore the icon.

Wouldn't it be better to add these here? @brianteeman

<svg aria-hidden="true" ...</svg>

[brianteeman]

Best to ask the accessibility team @carcam

[N6REJ]

Build in a condition if the .svg does not exist.

I do this kind of check at the Improvement Vote Plugin #31098, to avoid a warning message.

// Get the image
$image = HTMLHelper::_('image', 'plg_system_webauthn/webauthn.svg', '', '', true, true);

// If you can't find the image then skip it
if ($image === null)
{
	return;
}

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.

I don't believe you'll get a warning with my work?

[sandewt]

I don't believe you'll get a warning with my work?

Normally not, but if the image does not load / exists, you will get a warning message.

The following code prevents that. Try it. Change webauthn.svg eg to test.svg.

$image = HTMLHelper::_('image', 'plg_system_webauthn/webauthn.svg', '', '', true, true);

if ($image === null)
{
	return []; // break in case the svg does not exist
}

$image = JPATH_ROOT . substr($image, strlen(Uri::root(true)));

[N6REJ]

Build in a condition if the .svg does not exist.

I do this kind of check at the Improvement Vote Plugin #31098, to avoid a warning message.

// Get the image
$image = HTMLHelper::_('image', 'plg_system_webauthn/webauthn.svg', '', '', true, true);

// If you can't find the image then skip it
if ($image === null)
{
	return;
}

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.

idk how I missed it but your right it was broken. Fixed now.

[hans2103]

I'm not in the habit of repeating myself so please search and you will find it

Both comments does not help to solve this issue Brian.

I've tried to search and came with this comment... did you mean this one and the couple of comments following upon it?

#31079 (comment)

[N6REJ]

@hans2103 storm says adding aria to the <svg is not permissible there. Should I chalk this up to a mess up on storms part and do it anyway?
https://youtrack.jetbrains.com/issue/WEB-36210?_ga=2.236984006.739645025.1609423177-179347667.1594201852

[hans2103]

PHPStorm does not know if image is decorative or not. Might that be the trick?

[N6REJ]

@hans2103 after 2yrs you'd think storm would do a follow up on it all. I forget who our contact at storm is, i THINK its @joomla/marketing-communication-department

[brianteeman]

Does phpstorm obhect when you have role="presentation" and aria-hidden="true"

[N6REJ]

YES, role= or aria- it throws an error

[sandewt]

role="presentation" and aria-hidden="true"

YES, role= or aria- it throws an error

@N6REJ Do you mean the OR or an AND condition ?

[N6REJ]

PERIOD, it won't allow role or aria

[hvdmeer]

hans2103 after 2yrs you'd think storm would do a follow up on it all. I forget who our contact at storm is, i THINK its @joomla/marketing-communication-department

Try Roland Dalmulder, as far as I know he's the unofficial contact (or just has contact) with Jetbrains. Personally I don't think the Marketing department was involved with a developer tool.

[N6REJ]

@roland-d any help/input/feedback?

[roland-d]

@N6REJ What is the "error"? Here is what I see:

Support for aria- looks just fine.

Support for role seems fine too.

[N6REJ]

it's not in html its only in <svg like <svg aria-hidden="true" it says aria is not supported here. Same thing for role=
It was first reported 2 years ago https://youtrack.jetbrains.com/issue/WEB-36210?_ga=2.236984006.739645025.1609423177-179347667.1594201852

[roland-d]

@N6REJ Since it is confirmed by JB there is nothing else I can do at this point. Perhaps also link this issue in the JB issue?

[N6REJ]

I've decided to follow W3C and ignore storm till they get things sorted out, so lets get this tested and merged :D

[particthistle]

Went to apply the patch on Joomla 4 Beta 7 today and it came up with:

The patch could not be applied because it conflicts with a previously applied patch: administrator/modules/mod_login/tmpl/default.php

So there's a clash with something recently merged.

[N6REJ]

@particthistle I believe this means you already had a patch applied.
Git testing is not reporting an issue and everything was updated 5 days ago.

[particthistle]

Resetting patch tester component data cleared the error I was having. However @sandewt's review suggestion needs to be committed before I can test again now.

[particthistle]

I have tested this item ✅ successfully on 473cbb6

Tested successfully. Before and after applying the patch look identical on screen - it's the HTML that changes as well as the function changing to display the icon.

Icon now has aria-hidden="true" for both the front end and back end login buttons.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.}

[ghost]

I have tested this item ✅ successfully on 473cbb6

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31545.}

[HLeithner]

Thanks

[N6REJ]

ty