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

Optionally warn about missing <img width= height=> #757

Open
jidanni opened this issue Sep 26, 2018 · 5 comments
Open

Optionally warn about missing <img width= height=> #757

jidanni opened this issue Sep 26, 2018 · 5 comments
Milestone

Comments

@jidanni
Copy link
Contributor

jidanni commented Sep 26, 2018

Tidy should optionally warn if one forgets to put the width and height
attributes on tags. Warning about missing alt= is good, but
people also like their pages to load faster...

No none of the -access checks catch it either, (which makes sense.)
Tidy 5.2 (newest :-( ) on Debian.

@geoffmcl
Copy link
Contributor

geoffmcl commented Nov 1, 2018

@jidanni not a difficult feature request...

There is already a CheckIMG(doc,node) function, which performs the alt attribute check you mentioned, and the src...

It would be easy to add no width or height to output a message... need an option name, message, etc, etc - see adding OPTIONS for help doing this...

But really wonder about the use case... is there one?

There was a time, not so long ago, that good web content developers would take the trouble to provide a correctly sized source, maybe with a link to the full for separate viewing/download... and would guess that would still apply if faster page loading was the important criteria... where such a warning would not be needed... even be wrong, or at least very annoying...

Now you are suggesting they have become so lazy, time stressed, whatever... that they would forget to add size constraints to a large image... something they would see when testing before deployment, if they bother to do that anymore... so they would like to ask tidy to provide a reminder service... maybe, but seems a big stretch...

And I do not think adding size attributes, that adds an additional transformation to the process, would actually make the page load faster, but maybe, if say the video card/driver/display handling is a big component of the load time... but such a machine would certainly be better helped by supplying sized images...

But if someone were to take the trouble to code this, add patches, PR, then I suppose it could be considered... given a use case...

What do others think... thanks...

@jidanni
Copy link
Contributor Author

jidanni commented Nov 1, 2018 via email

@geoffmcl
Copy link
Contributor

geoffmcl commented Nov 1, 2018

@jidanni thanks for the feedback, but it seems you have not yet fully specified this feature request... exactly what is it supposed to do? Its purpose? What do you want it to do? Show?

Readers need to know there was a big image there vs. a tiny 1x1 image there.

While that might be accepted as true, and tidy does try to be helpful, informational, even educational, how does this FR to Optionally warn about missing <img width= height=> help in this?

Or are you adding that even if present, this option would warn, showing the values of these attributes... but that still offers no clue about the actual image size... do not understand... confused...

Some pseudo code, untested, just thoughts, on how it might look -

void CheckIMG( TidyDocImpl* doc, Node *node ) {
    /* some new option, like 'check-img-sizes', 'show-img-sizes', or ... */
    if (nodeIsIMG(node) && cfg(doc, TidyChkImgSizes))
    {
        AttVal *ph = TY_(AttrGetById)(node, TidyAttr_HEIGHT);
        AttVal *pw = TY_(AttrGetById)(node, TidyAttr_WIDTH);
        if (!ph || !pw)
        {
            /* Is #757 - img: report a missing size attrib - 
               could separate to which one, or both... 
               details missing in 'spec' */
            printf("Info:Warning:Error: img tag missing height and/or width attribute...\n");
        }
        else
        {
            /* is anything to be done in this case? */
            printf("[type?]: img tag has width=%s, and height=%s...\n", pw, ph );
        }
    }

Have marked it a Feature Request.

Need someone to document the full option specs, discuss, agree, code it, and present patches, PR, and then, as stated, hopefully with more use case support, it will be considered...

Look forward to more... thanks...

@ler762
Copy link
Contributor

ler762 commented Nov 1, 2018

And I do not think adding size attributes, that adds an additional transformation to the process, would actually make the page load faster ...

I read about that a long time ago; it might even still be relevant/true :) Specifying the image size allows the page render process to leave a "hole" where the image goes. Then when the image is downloaded the browser just plops it into the correct place & it's done. If the image tag doesn't have a height/width attribute the browser has to wait until the image is downloaded to figure out how much room the image takes up in the page. So the browser either stalls until it gets the image or keeps going with displaying the page until the image is downloaded and re-draws the page again (re-flow in firefox?) with the new image size info.

@jidanni
Copy link
Contributor Author

jidanni commented Nov 1, 2018

@geoffmcl I don't think tidy is a dead link checker, or should look for image files and tell the user the size to put in. I just want in to tell me, in addition that I should add an "alt=", I would optionally usually want to know that I forgot width= and/or height=. (And no, no AI needed to tell me what the alt= should say :-) )

And yes saying "missing ... and/or ..." is good enough, as if the user just forgot one of width or height, he already needs more help than tidy can provide :-)

@balthisar balthisar added this to the 5.9 milestone Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants