1669090 - simple Scratch project crashes on Firefox Nightly 83.0a1

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[[e-mail]]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

I visit this URL:
https://scratch.mit.edu/projects/432397436/fullscreen/
on Firefox Nightly 83.0a1. This is a URL to open a simple Scratch project in full screen.
The page loads and shows a button with a green flag. I click this flag to run the project.

Actual results:

Scratch shows a message:
"Ups! Coś poszło nie tak. Przykro nam, ale wygląda na to, że program Scratch uległ crashowi. Ten błąd został automatycznie zgłoszony do zespołu firmy Scratch. Odśwież stronę, aby spróbować ponownie.".
This message means: "Oops! Something went wrong. Sorry, but Scratch seems to have crashed. This error was automatically reported to the Scratch team. Please refresh the page to try again.".

Expected results:

This message should not be displayed. It is not displayed when I open this URL in Chromium.
I reported this problem to Scratch developers as well:
https://scratch.mit.edu/discuss/topic/444579/?page=1#post-4473003

Comment 1

[Tooru Fujisawa [:arai]]

The error happens because of the following code throws DOMException: Index or size is negative or greater than the allowed amount:

this._context.drawImage(this._cachedImage, 0, 0)

where this._cachedImage is

<img src="data:image/svg+xml;utf8,%3Csvg%20version%3D%221.1%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%20xmlns%3Axlink%3D%22http%3A%2F%2Fwww.w3.org%2F1999%2Fxlink%22%20width%3D%220.45871%22%20height%3D%226.51376%22%20viewBox%3D%220%2C0%2C0.45871%2C6.51376%22%3E%3Cg%3E%3Cg%20data-paper-data%3D%22%7B%26quot%3BisPaintingLayer%26quot%3B%3Atrue%7D%22%20fill-rule%3D%22nonzero%22%20stroke-linecap%3D%22butt%22%20stroke-linejoin%3D%22miter%22%20stroke-miterlimit%3D%2210%22%20stroke-dasharray%3D%22%22%20stroke-dashoffset%3D%220%22%20style%3D%22mix-blend-mode%3A%20normal%22%3E%3Cpath%20d%3D%22M237.94309%2C206.75856v-6.51376h0.45871v6.51376z%22%20stroke-width%3D%220%22%20fill%3D%22%23ff0000%22%20stroke%3D%22none%22%20transform%3D%22matrix(1%2C0%2C0%2C1%2C-237.94309%2C-200.2448)%22%2F%3E%3C%2Fg%3E%3C%2Fg%3E%3C%2Fsvg%3E">

the source image is the following SVG, that has width with floating number less than 0.5, that seems to be rounded to 0 inside drawImage:

<svg version="1.1"
     xmlns="http://www.w3.org/2000/svg"
     xmlns:xlink="http://www.w3.org/1999/xlink"
     width="0.45871"
     height="6.51376"
     viewBox="0,0,0.45871,6.51376">
  <g>
    <g data-paper-data="{&quot;isPaintingLayer&quot;:true}"
       fill-rule="nonzero"
       stroke-linecap="butt"
       stroke-linejoin="miter"
       stroke-miterlimit="10"
       stroke-dasharray=""
       stroke-dashoffset="0"
       style="mix-blend-mode: normal">
      <path d="M237.94309,206.75856v-6.51376h0.45871v6.51376z"
            stroke-width="0"
            fill="#ff0000"
            stroke="none"
            transform="matrix(1,0,0,1,-237.94309,-200.2448)"/>
    </g>
  </g>
</svg>

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: minimal testcase

if image element's src is set to SVG that has non-integer width/height, the rounding algorithm differs between firefox and chromium/safari.

if width < 0.5, img.width becomes 0 on Firefox, while it becomes 1 on others chromium,
and that seems to be the reason why it hits IndexSizeError here
https://searchfox.org/mozilla-central/rev/4352fb7b0d17c1febff9569ed311e0e42c93093e/dom/canvas/CanvasRenderingContext2D.cpp#4506

void CanvasRenderingContext2D::DrawImage(const CanvasImageSource& aImage,
                                         double aSx, double aSy, double aSw,
                                         double aSh, double aDx, double aDy,
                                         double aDw, double aDh,
                                         uint8_t aOptional_argc,
                                         ErrorResult& aError) {
...
  if (aSw == 0.0 || aSh == 0.0) {
    aError.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
    return;
  }

Comment 3

[Boris Chiou [:boris]]

Thanks for providing the minimal test, Tooru. CanvasRenderingContext2D::DrawImage is updated by GFX team recently, and I'm not sure how to handle this case, so move the component to graphics for now.

Comment 4

[Tooru Fujisawa [:arai]]

Other than the img element's size, there seems to be another issue in drawImage implementation.
the spec says the following:

https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-drawimage

When the drawImage() method is invoked, the user agent must run these steps:
...
  4. ...
      If the sx, sy, sw, and sh arguments are omitted, then they must default to 0, 0, the image's intrinsic width in image pixels, and the image's intrinsic height in image pixels, respectively. 
      ....
  5. If one of the sw or sh arguments is zero, then return. Nothing is painted.

So, if image's width is 0, it should just return without doing anything.
Throwing IndexSizeError seems to be wrong.

Tested again with the testcase, and Safari returns 0 for width < 0.5, but still doesn't throw,
and the above seems to be the reason of the difference.

Comment 5

[Tooru Fujisawa [:arai]]

the above IndexSizeError is added by bug 629876, and looks like the spec has been modified since then

Comment 6

[Tooru Fujisawa [:arai]]

https://searchfox.org/mozilla-central/source/testing/web-platform/meta/html/canvas/element/drawing-images-to-the-canvas/2d.drawImage.zerosource.html.ini
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/html/canvas/offscreen/drawing-images-to-the-canvas/2d.drawImage.zerosource.html.ini
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/html/canvas/element/drawing-images-to-the-canvas/2d.drawImage.zerosource.html
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/html/canvas/offscreen/drawing-images-to-the-canvas/2d.drawImage.zerosource.html

there's web-platform testcase, with expected fail for this.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1669090 - Do not throw when sw/sh of drawImage is zero. r?jrmuizel!

Comment 8

[Tooru Fujisawa [:arai]]

Opened https://github.com/web-platform-tests/wpt/pull/26246 for the test fix

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df39036a3a68
Do not throw when sw/sh of drawImage is zero. r=jrmuizel

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/df39036a3a68