apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
696 stars 158 forks source link

HTML Width and Height are auto removed #2068

Open mitko-janeski opened 3 years ago

mitko-janeski commented 3 years ago

Hello guys I want to report a problem, in my HTML file I have

img src="file.jpg" alt="File Name" class="mr-2" width="32" height="32"

File after pagespeed-mod img src="data:image/webp;base64,UklGRtYBAABXRUJQVlA4IMoBAAAwCACdASogACAAPm0skUWkIqGYBABABsS1AE6ZdnFfCB2ojfauANBN9TzP583ei2nuKFTF1HroFeSRRwtmfLKZxZQg1Wp2VjEuAAD+/9TKg8t7Ku+x4b//BNO1dxn6ev1NxZdU8UCJJ9p+Fm+5nN9/997//uPwidX/Hb/RmJW/L2d8mKv9/8iuFbWfe/prEgm7bqLgNR55KnD87txBM/RPnP0Vbc/9A1eyHw1NPbxX/vheNsd0n4za+DC5SohhFU/AYR6dBy1GcPtjtWkRwemzf+MgkMfM8u1dASE9F3TgzFDMXCDt6Jf2dfdQzZ+fMrRZuvwA5nFRN93SYQmR4f/GxVo5339/8f0klPQLdOyfVN/r/18dnofdoYZoclbnI4La0mr1d6Qbfjt13+ZR3/jsqNP/pTqzBae2Bydd/H98ON6D/FUl8V1X+Ntb9dGGzb/gFVZdo07h9epurkmMpmxGhxKh6F2d3QTSnCWlz7B5J7o0+Z9+lntP8ZNKnbMi5tSLzmblUi1ROhn5wAKD2vLU1nS+eoz0gdhU5V1L6vZz0FqcG2r7K3e4wBTV1HMLIXnz8QSMEtdfhmVqRAG7wubDSgB5bsHUMEAAAA==" alt="File Name" class="mr-2" data-pagespeed-url-hash="319205374" data-pagespeed-onload="pagespeed.CriticalImages.checkImageForCriticality(this);" onload="var elem=this;if (this==window) elem=document.body;elem.setAttribute('data-pagespeed-loaded', 1)" id="pagespeed_img_Ny12gAmBpH1" data-pagespeed-loaded="1"

As you can see width and height attributes are removed and inside Google Lighthouse Report I get this: "Image elements do not have explicit width and height"

How can I resolve this problem ?

centos 7 apache 2.4 browser chrome pagespeed: 1.13.35.2 via easy apache

Lofesa commented 3 years ago

The images are inlined by the filter: ModPagespeedEnableFilters inline_images you can disable this filter and then you got "normal" url.

Anyway thi issue must be reported to Ligthhouse, because data:uris spec doný have dimensions.

Can you add additional info to this LH issue? https://github.com/GoogleChrome/lighthouse/issues/12529

patrickhulce commented 3 years ago

Hi there! I think there's some confusion @Lofesa.

because data:uris spec doný have dimensions.

Lighthouse doesn't want width/height as part of the data URI, it wants width/height attributes as described in the OP's example that pagespeed mod appears to remove. Even if a data URL loads much faster, there is still a decode component that can be async and lead to layout shifts, seems worthwhile to preserve them at least.

Lofesa commented 3 years ago

Hi @patrickhulce As far as I can see in the PR##12120 the unsized non-network svg are ignored, Is not the same with inlined images? Another way to set image size is with css, Is this way acceptable for LH?

patrickhulce commented 3 years ago

Is not the same with inlined images?

It is not. Encoded images formats like JPEG/WebP/PNG need to be decoded on a separate background thread, so layouts happen in between. In practice it appears that the layout is not affected on fast machines for JPEG on Chromium (perhaps because Chromium at least waits for the header with dimensions to be decoded? I'm not sure). If you have other knowledge that this behavior is actually guaranteed in all browsers, happy to open new discussion over there about excluding all non-network images.

Another way to set image size is with css, Is this way acceptable for LH?

Sure, any method of fixed sizing will work for Lighthouse, but it appears that the OP was already setting them via attributes and this mod removed them. Seems reasonable to at least keep attributes the developer intended, even if they aren't strictly necessary for this specific image on Chrome, no?