CSS position is not set, defaulting to static, therefore still affects layout (breaks some styles)

Do you have some reference (link) to those styles?

For some reason, alt is set to "cron", when it should be ""; alt attribute is supposed to be an accessible alternative representation to the user agent, so should be set to empty string if the element is not supposed to be visible

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Img
The alt attribute holds a text description of the image, which isn't mandatory but is incredibly useful for accessibility — screen readers read this description out to their users so they know what the image means. Alt text is also displayed on the page if the image can't be loaded for some reason: for example, network errors, content blocking, or linkrot.

Do you have some reference (link) to those styles?

ProLight is one of them. But the issue is with core, not the style — invisible elements should never affect layout.

The alt attribute holds a text description of the image, which isn't mandatory but is incredibly useful for accessibility — screen readers read this description out to their users so they know what the image means. Alt text is also displayed on the page if the image can't be loaded for some reason: for example, network errors, content blocking, or linkrot.

Yes, setting the alt image is important; but it should only be set to a non-empty string when the image has content that's meaningful to the user. A lot of ink has already been spilled on this topic — see e.g. any of the following:

• https://osric.com/chris/accidental-developer/2012/01/when-should-alt-text-be-blank/
• https://accessibility.oit.ncsu.edu/it-accessibility-at-nc-state/developers/accessibility-handbook/alternative-text/#2
• https://www.w3.org/WAI/tutorials/images/decorative/
• https://stackoverflow.com/questions/23526791/when-is-it-acceptable-to-leave-the-alt-text-empty-on-an-html-image-if-ever

The problem is where the image is 1px X 1px, which has been made for a good reason invisible. The problem lies probably in that style... IMHO.

HTML attributes (if empty) can not be in the core. Like alt="". BTW.

I think this was a hack to run the cron jobs asynchronously so it doesn't interfere with page load.
Would there be a problem if the image was removed and simply changed with a fetch call from javascript?

HTML attributes (if empty) can not be in the core. Like alt="". BTW.

In that case, the style rules for core are directly violating W3C WAI accessibility criteria in this specific case.

Or do you mean the HTML should be <img alt ...> instead of <img alt="" ...>? If so, happy to make that change, as the two are semantically equivalent.

I think this was a hack to run the cron jobs asynchronously so it doesn't interfere with page load.
Would there be a problem if the image was removed and simply changed with a fetch call from javascript?

Yeah, that's what I figured. Don't see why that wouldn't do the trick, other than that fetch isn't supported in IE11, and IIRC phpBB 3.3 officially supports as low as IE9 (🤯🤯🤯).

I think this was a hack to run the cron jobs asynchronously so it doesn't interfere with page load. Would there be a problem if the image was removed and simply changed with a fetch call from javascript?

What if someone disabled JS?

HTML attributes (if empty) can not be in the core. Like alt="". BTW.

In that case, the style rules for core are directly violating W3C WAI accessibility criteria in this specific case.

Or do you mean the HTML should be <img alt ...> instead of <img alt="" ...>? If so, happy to make that change, as the two are semantically equivalent.

It should do, I'll leave the answer to the Devs though.

I think this was a hack to run the cron jobs asynchronously so it doesn't interfere with page load. Would there be a problem if the image was removed and simply changed with a fetch call from javascript?

What if someone disabled JS?

Upon testing, the worst case scenario here is that the cron job doesn't run on that specific page load; it'll still run the next time a user not using IE or disabling JS visits. In other words, the worst case is as if (for the purpose of the cron job) that individual site visit hadn't taken place.

Is this a worthwhile tradeoff for the reduced hackiness? My gut says yes, but I'd like to get others' thoughts.

There's also the option of XMLHttpRequest, which is IE compatible but would still require JS being enabled.

Not true our guidelines have been updated https://area51.phpbb.com/docs/dev/master/development/html/guidance.html

No slashes and " are required. We follow idiomatic standards for html.

We also no longer support IE so not sure thats an issue. As 3di pointed out all html should be avoided in core.

Wether the cron not always running is acceptable, I will let others weigh in on. But my vote is on a js solution.

@hanakin this is a special case. That cron should always be running (ready to). And JS is not an acceptable solution, IMHO.

And I pointed out that INLINE STYLING should be avoided in core in this case, please read the proposed code, there are not so many ways to do what you want if not re-factor the whole stuff. But since this is a PR against 3.3.x and not master, it will do.

Yup agree with @3D-I on this.

@hanakin this is a special case. That cron should always be running (ready to). And JS is not an acceptable solution, IMHO.

And I pointed out that INLINE STYLING should be avoided in core in this case, please read the proposed code.

Yup agree with @3D-I on this. Please pursue a non JS solution.

OK, next question: I agree in principle that all HTML should be kept in templates, so an ideal solution would look something like this*:

phpBB/viewforum.php and phpBB/controller/helper.php:

$template->assign_var('CRON_TASK_URL', $url);

phpBB/styles/prosilver/template/overall_footer.html:

{% if not S_IS_BOT %}
	{#
		Runs the cron task by triggering server request to the URL on load.
		`img` is preferred over JS to ensure maximum compatibility.
		`sr-only` hides visually, `aria-hidden` hides from screen-readers;
		`hidden` or `display: none` would prevent the task from running.
	#}
	<img src="{CRON_TASK_URL|e('html_attr')}" alt="" width="1" height="1" class="sr-only" aria-hidden="true">
{% endif %}

However, this would leave RUN_CRON_TASK undefined, so any style that used a different overall_footer.html that expected RUN_CRON_TASK but not CRON_TASK_URL would fail to run any cron tasks at all!

This seems like a much more far-reaching consequence than the lack of support for IE or people turning off JS, both of which are tiny demographics.

That leaves 3 alternatives that I can think of:

1. Assign both CRON_TASK_URL (for use by new styles) and RUN_CRON_TASK (for legacy use).
2. Continue to assign only RUN_CRON_TASK, with the old HTML-in-PHP solution, leaving a comment in the code to explain why this is necessary in this case.
3. Add a partial cron.html.twig under phpBB/styles/all/template/ (or somewhere else?) that contains the img tag alone, then assigning the output of that template to RUN_CRON_TASK. Perhaps the rendering logic of this could even be done inside phpBB/phpbb/cron/task/wrapper.php, so that it'd be easily accessible from both phpBB/viewforum.php and phpBB/controller/helper.php; if not, I'm not really sure where it should go.

My vote goes to #‍3, as it seems the most "correct" while also being robust. Thoughts?

* I kept alt="" instead of simply alt as that's the style used in an example in the style guide @hanakin linked to.

Just a note (again) the alt text is also displayed on the page if the image can't be loaded for some reason: for example, network errors, content blocking, or linkrot.

Just a note (again) the alt text is also displayed on the page if the image can't be loaded for some reason: for example, network errors, content blocking, or linkrot.

You've mentioned this before, but I'm not sure what its significance is? The tag is styled invisibly, so it won't be visible either way; setting alt to empty string simply makes the semantics/accessibility characteristics consistent with the visual styling.

In no case should an img tag that's simply used to make a request to a server endpoint have a contentful alt attribute; it's specifically intended to be invisible to the user agent (just as it would be if a fetch request was substituted).

It is very known that you can see the word "cron" in case of issues. The tag is not styled invisibly, the IMG is 1px X 1px which makes it invisible to the human eye.

It is very known that you can see the word "cron" in case of issues. The tag is not styled invisibly, the IMG is 1px X 1px which makes it invisible to the human eye.

Displaying the text "cron" via alt attribute in the site footer is a really bad method of error reporting, because

1. It's confusing ("wtf is a cron?");
2. It doesn't mention anything about it being an error ("huh, I guess they just decided to display a random word there");
3. It's not localized ("cron 到底什么鬼？");
4. It doesn't give the user any information on what (if anything) they should do about it; and
5. It's inaccessible, because it abuses the alt attribute for something it was never intended for. alt displays to screen readers whether or not the image has loaded correctly.

If we want to add user-visible error reporting, the proper way would be via onerror or addEventListener('error', ...), along with a phpbb.alert(localizedTitle, localizedMessage), then adding the relevant translations. However, it's not clear to me that this kind of thing should ever be visible to ordinary forum users, because it's not their concern; rather, it should be handled in the back end, perhaps by logging when a request is made that renders the cron tag but fails to hit the corresponding endpoint within a couple of seconds.

I'd humbly suggest that the question of error handling stays out of scope for this particular PR, which relates to styling and accessibility. Perhaps someone else can create another PR for the logging issue if that isn't already handled satisfactorily.

Agreed. The alt tag is for accessibility and should not be used for the cron job image. I wonder if the (eventual?) solution should to just make the cron task serve a blank image when used in this way.

The display of the raw text of the ALT attribute is perfectly fine in those situations as per
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Img

That's it by design btw, IMHO. In this special case.

If it appears it means your style is badly coded or your board is not fully functional, that's it.

The display of the raw text of the ALT attribute is perfectly fine in those situations as per https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Img

That's it by design btw, IMHO. In this special case.

Perfect example of why to leave it blank. From that page:

The alt attribute holds a text description of the image, which isn't mandatory but is incredibly useful for accessibility — screen readers read this description out to their users so they know what the image means.

If you're using a text reader and it reads "cron" as part of the page it would be very weird. If it's left blank it would not adversely affect anything so there should be no reason to not make the change.

So, let's go to the origin of it and blame it. Let's see what the devs will say.
In any case it can not be left blank, dead code.

it can not be left blank

This is patently false. It can be left blank. I'll quote from the link you provided:

Setting this attribute to an empty string (alt="") indicates that this image is not a key part of the content (it's decoration or a tracking pixel), and that non-visual browsers may omit it from rendering.

That's not a part of the guidelines though, I recall also extensions being denied for such a "mistake".
You guys should really make peace with yourselves, not that one day one says something and then it turns out not to be true. Small parenthesis.

In the rare event that there is something in the phpBB style rules or conventions that directly contradicts W3C's Web Content Accessibility Guidelines, I'd suggest that it'll generally be phpBB that needs to change and not the WCAG... phpBB's great and all, but it's not as authoritative as the standards organization for the web itself 😉

In the rare event that there is something in the phpBB style rules or conventions that directly contradicts W3C's Web Content Accessibility Guidelines, I'd suggest that it'll generally be phpBB that needs to change and not the WCAG... phpBB's great and all, but it's not as authoritative as the standards organization for the web itself 😉

There is a front-end developer team leader in charge, on the graphs. 😶

Changes made based on what I outlined in option #‍3 of #6376 (comment)

@lionel-rowe I love your last commit. 💯 I have no time to review it now, I saw something though that can be improved, anyway... I will post here my thoughts maybe tonight or tomorrow.

Just checked myself and it seems like tests will still need to be updated.

@lionel-rowe Your commits need to have this format:

[ticket/16977] Fix cron-job img tag layout and accessibility

PHPBB3-16977

You can do it like this or if you want i can update it for you

Tests and commit messages should now be fixed.

I've created a master version of your PR that will ease merging the PR: #6381
Once the builds have ran through I'll proceed with merging everything.