Closed Bug 1602047 Opened 5 years ago Closed 5 years ago

Firefox no reset height for images with empty src attrib

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Version:
71 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla73
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- wontfix
firefox72 --- verified
firefox73 --- verified

People

(Reporter: krystian3w, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

broken size image in Firefox 71
14.06 KB, text/html
Details
css hotfix.htm
777 bytes, text/html
Details
Bug 1602047 - Cleanup nsImageFrame::UpdateIntrinsic{Size,Ratio}. r=tnikkel
47 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta+
Details | Review
Bug 1602047 - For <img src=""> (no current request), do not use mapped aspect ratio. r=tnikkel
47 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

Open this demo: https://jsfiddle.net/jpz8nfxL/
In right down square you should see image element with "border", even though the photo size was reset to 0 jheight before.

Actual results:

CSS height: auto; no longer reset size to 0 value if atrrib src for image is empty

Expected results:

Still reset size image to 0 height.
Like Firefox 70 and any Chromium / Legacy Edge (EdgeHTML engine).

Attached file css hotfix.htm —

manually reset height for Firefox 71+:

@supports (grid-template-columns: subgrid) {
  img[src=""][height] {
      height: 0;
}
}
Comment on attachment 9114215 [details] broken size image in Firefox 71 ><!DOCTYPE html> ><html><head> > <meta http-equiv="content-type" content="text/html; charset=UTF-8"> > <title></title> > <meta http-equiv="content-type" content="text/html; charset=UTF-8"> > <meta name="robots" content="noindex, nofollow"> > <meta name="googlebot" content="noindex, nofollow"> > <meta name="viewport" content="width=device-width, initial-scale=1"> > <style id="compiled-css" type="text/css"> > img { > max-width: 100%; > height: auto; >} > </style> > > </head> ><body> > <a href="#" rel="home"> > <img class="custom-header" src="" alt="" scale="0" width="1030" height="680"> ></a> > ></body></html>
Attachment #9114215 - Attachment description: broken size img.htm → broken size image in Firefox 71
Component: Untriaged → Layout
Product: Firefox → Core

This is working as expected per the changes discussed in bug 1547231 / https://github.com/WICG/intrinsicsize-attribute/issues/16.

The width and height attribute causes the image to have a defined aspect ratio, despite the src being broken.

Are you aware of this breaking any site? We can probably still change behavior here but it seems to be the intended thing if you specify the width and height of the image that the size of the image is computed relative to that.

Breaking this blog:

https://davidpohl.wordpress.com/

source (Polish): https://mozillapl.org/forum/viewtopic.php?p=245707#p245707

And the creator may not necessarily have time to remove the graphics from the template or hide it in CSS (and Wordpress surpasses it).

Alright, I'll take a look at this.

Assignee: nobody → emilio
Status: UNCONFIRMED → NEW
Ever confirmed: true
Regressed by: 1547231
Has Regression Range: --- → yes

(Just so I don't forget)

Flags: needinfo?(emilio)

This works in chrome because they don't event create an image box for this case,
which is totally against the spec.

The spec doesn't consider an image with a null image request, but our behavior
changes in some other places as well because of it...

Depends on D56367

Flags: needinfo?(emilio)
Priority: -- → P2
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5178a33eae28 Cleanup nsImageFrame::UpdateIntrinsic{Size,Ratio}. r=tnikkel https://hg.mozilla.org/integration/autoland/rev/bc309b2cd9e7 For <img src=""> (no current request), do not use mapped aspect ratio. r=tnikkel
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20701 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR merged by moz-wptsync-bot

https://hg.mozilla.org/mozilla-central/rev/5178a33eae28
https://hg.mozilla.org/mozilla-central/rev/bc309b2cd9e7

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox73: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla73

Given that this has been around since Fx69, is this worth Beta backport consideration?

status-firefox71: --- → wontfix
status-firefox72: --- → fix-optional
status-firefox-esr68: --- → unaffected
Flags: needinfo?(emilio)

Comment on attachment 9114555 [details]
Bug 1602047 - Cleanup nsImageFrame::UpdateIntrinsic{Size,Ratio}. r=tnikkel

Beta/Release Uplift Approval Request

  • User impact if declined: comment 0
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0
  • List of other uplifts needed: none
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Minor change to not apply a new feature to images with no src attribute.
  • String changes made/needed: none
Flags: needinfo?(emilio)
Attachment #9114555 - Flags: approval-mozilla-beta?
Attachment #9114556 - Flags: approval-mozilla-beta?

Slightly confused because comment 0 says this is new in 71, but the regressing bug was in 69.

(In reply to Julien Cristau [:jcristau] from comment #16)

Slightly confused because comment 0 says this is new in 71, but the regressing bug was in 69.

The feature introduced by the regressing bug was only enabled on release for 71.

(In bug 1585637 in particular)

Comment on attachment 9114555 [details]
Bug 1602047 - Cleanup nsImageFrame::UpdateIntrinsic{Size,Ratio}. r=tnikkel

Aha, thanks. Probably makes sense to uplift in that case; approved for 72.0b8.

Attachment #9114555 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9114556 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Verified as fixed on Firefox 72 beta 9 and on the latest Nightly 73.oa1 (build ID: 20191222215627) on Windows 10 x64, Ubuntu 18.04 and Mac OS X 10.15.

QA Whiteboard: [qa-triaged]
status-firefox72: fixed → verified
status-firefox73: fixed → verified
Flags: qe-verify+
Regressions: 1621333

I suppose problem is back on same WordPress blog (https://davidpohl.wordpress.com/), new specification revert these repair added to Firefox 72-85?

I fastly checked and something new added into Firefox 83+ reverted/glitched:

https://hg.mozilla.org/releases/mozilla-beta/rev/a0d750de744a
https://hg.mozilla.org/releases/mozilla-beta/rev/df4c26f082cf

Can you elaborate? The image also looks big in other browsers.

Can you elaborate?

  • In Chrome 89 frame with information about broken image is much is smaller:

    https://jsfiddle.net/4ch5t3wL/ - https://i.imgur.com/ReNuPPg.png browser don't use height="680" for frame and I see ~20 pixels height between lines. I don't know what generate margin/padding bottom equal ~660 pixels.

  • Firefox 83+ - frame is huge like as 680 height attrib: https://i.imgur.com/aW5O4WD.png

  • Safari 14 - reserves size equal 1 line of text (~16 px height) - maybe due used <a> ... </a> element.

The image also looks big in other browsers.

So it's all about how much space the element now reserves and not the frame size reshuffling for an empty src attribute or a network error accessing the file.

Please file a new bug for comments 23/25.

status-firefox86: affected → ---
status-firefox87: affected → ---
status-firefox88: affected → ---

OK, cloned and I wait for feedback by people better know how should works based on documentation for html/css.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: