Re: Bug in Acid3 test #72 (race condition when images decode asynchronously)

On Thu, 22 Dec 2016 01:13:46 +0100, Daniel Holbert <dholbert@mozilla.com>  
wrote:

> Hi Hixie,
>
> tl;dr: here's my requested acid3 test modification:
>   https://bug1325080.bmoattachments.org/attachment.cgi?id=8820958
>
> BACKGROUND
> ==========
> We at Mozilla got a bug report[0] for a regression in our Acid3 test
> score, but in fact I think it's an incorrect/unstated dependency in the
> test, and I think it should be fixed in the test itself.
>
> This is for subtest #72 ("dynamic modification of <style> blocks' text
> nodes"), and it's just an issue with one of the initial-conditions
> sanity-checks in that test.
>
> ANALYSIS/EXPLANATION:
> =====================
> Basically, Acid3 test #72 adds a styled <img> element to the document
> (technically this happens in the "finally{}" for test #71), and then
> synchronously reads img.height in JS, and asserts that it matches the
> specified CSS "height: 10px": This assertion is:
>
>   assertEquals(doc.images[0].height, 10,
>     "prerequisite failed: style didn't affect image");
>
> I claim this assertEquals() isn't entirely justified.  If the image data
> isn't cached & the image-decoder hasn't kicked into action yet, then the
> image will still be in its initial "not available" state, as defined in
> the HTML5 spec [1]. And since the 'src' attribute is set and the 'alt'
> attribute is set to the empty string, that means "the element represents
> nothing" [2].  And elsewhere, the spec says an <img> element that
> "represents nothing" is supposed to be rendered as an empty inline
> element [3].  And the "height" CSS property has no effect on an empty
> non-replaced inline element [4].  So, the assertion about "height"
> applying is invalid, in this case where the image hasn't advanced past
> its not-available state yet.  The assertion is only valid once the image
> has become "available".
>
> SUGGESTED FIX:
> ==============
> Could you edit Acid3 subtest #72 to check the img's "complete" flag
> (HTMLImageElement.complete), and return "retry" if that flag is false?
> It looks like the harness will handle this nicely and retry the test
> after a short timeout. As noted above, I've got a patch to do this here:
> https://bug1325080.bmoattachments.org/attachment.cgi?id=8820958
> I tested this locally in Firefox & Chrome, and it seems to work nicely.
>
> (According to MDN, the "HTMLImageElement.complete" flag is supported in
> all browsers [5].)
>
> Thanks!
> ~Daniel
>
> [0] https://bugzilla.mozilla.org/show_bug.cgi?id=1325080
>
> [1]
> https://html.spec.whatwg.org/multipage/embedded-content.html#img-available
> "an image request is initially unavailable"
>
> [2]
> https://html.spec.whatwg.org/multipage/embedded-content.html#the-img-element:attr-img-src-5
>
> [3] https://html.spec.whatwg.org/multipage/rendering.html#images-3
> (Technically this chunk of spec requires that "the user agent does not
> expect this to change". But the spec doesn't say *what* to do if the
> user agent *does* expect this to change, or what "expects this to
> change" actually means.)

This part seems important. I think it was meant to allow UAs to apply CSS  
width/height so layout can be stable during page load, but as you point  
out this is not well specified in HTML. Can you file an issue, please?


> [4] https://www.w3.org/TR/CSS21/visudet.html#propdef-height
> "This property does not apply to non-replaced inline elements"
>
> [5]
> https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement#Browser_compatibility
>


-- 
Simon Pieters
Opera Software

Received on Thursday, 22 December 2016 14:24:10 UTC