1579402 - No action triggered when setting a very large (21600x18000) PNG image as desktop background via context menu

Status: NEW

(Firefox :: Shell Integration, defect, P3)

Description

[Vlad Lucaci, QA (:vlucaci)]

Affected versions

• 69.0
• 70.0b4
• 71.0a1

Affected platforms

• Windows 10x64
• Ubuntu 16.04x64

Steps to reproduce

1. Launch FF.
2. Access https://tinyurl.com/y4n2mx4e and wait for the image to load.
3. Right click the image and select "Set as desktop background" option.

Expected result

• Image is set as desktop background on the OS.

Actual result

• No action is triggered after selecting the "Set as desktop background" option.

Regression range

• Will return with regression ASAP.

Additional notes

• Other images with PNG format (as well as JPG format) are successfully set as OS desktop image via the context menu option.

Comment 1

[Vlad Lucaci, QA (:vlucaci)]

Hello,
I have managed to pull a regression range for this issue.
Although , for some reasons builds contained in the period between the first bad and the good are not launching the picture at all.

• First bad: 2015 09 07
• Last good: 2015 03 09
• Pushlog:https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=eab4a81e4457&tochange=df817a53e24bbacd62347cb9873d86039448b666
• Potentially regressed by: 1187569, 1191090

Comment 2

[:Gijs (he/him)]

The regression range and bugs listed as regressors actually suggests this is an imglib issue. Was there some other reason that made you move this out of imglib?

Comment 3

[:Gijs (he/him)]

Never mind, looks like this is likely to have to do with the fact that the e10s implementation here uses a data URI at https://searchfox.org/mozilla-central/rev/250f5cc9fb8bdcbb6b23d2a06acfd48addb2f99b/browser/actors/ContextMenuChild.jsm#248, which fails for an image this large.

I think we probably need to find some other way of passing this information to the parent... We can use a blob URI, but frankly, even painting this whole thing to a canvas to get a blob URI out of it seems dumb. Leaving ni to ask a different question: what better options have we got?

Comment 4

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #3)

Never mind, looks like this is likely to have to do with the fact that the e10s implementation here uses a data URI at https://searchfox.org/mozilla-central/rev/250f5cc9fb8bdcbb6b23d2a06acfd48addb2f99b/browser/actors/ContextMenuChild.jsm#248, which fails for an image this large.

I think we probably need to find some other way of passing this information to the parent... We can use a blob URI, but frankly, even painting this whole thing to a canvas to get a blob URI out of it seems dumb. Leaving ni to ask a different question: what better options have we got?

Sigh, putting ni back...

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to :Gijs (he/him) from comment #3)

Never mind, looks like this is likely to have to do with the fact that the e10s implementation here uses a data URI at https://searchfox.org/mozilla-central/rev/250f5cc9fb8bdcbb6b23d2a06acfd48addb2f99b/browser/actors/ContextMenuChild.jsm#248, which fails for an image this large.

I think we probably need to find some other way of passing this information to the parent... We can use a blob URI, but frankly, even painting this whole thing to a canvas to get a blob URI out of it seems dumb. Leaving ni to ask a different question: what better options have we got?

Not really my area of expertise, but even just sending the compressed image source file would be better.

Comment 6

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Working on finding a triage owner for this component.
Andrew, do you have any suggestions for who might work on this?

Comment 7

[Andrew Overholt [:overholt]]

(In reply to :Gijs (he/him) from comment #2)

The regression range and bugs listed as regressors actually suggests this is an imglib issue. Was there some other reason that made you move this out of imglib?

I am curious about the above, too. If it's indeed an imagelib issue then I think we have our winner for who could work on it.
If it's not an imagelib issue (like I guess the data:// -> Blob suggestion), I'd ask Mike Conley but won't needinfo him here until we determine whether or not the root cause is in imagelib.

Comment 8

[:Gijs (he/him)]

(In reply to Andrew Overholt [:overholt] from comment #7)

(In reply to :Gijs (he/him) from comment #2)

The regression range and bugs listed as regressors actually suggests this is an imglib issue. Was there some other reason that made you move this out of imglib?

I am curious about the above, too. If it's indeed an imagelib issue then I think we have our winner for who could work on it.
If it's not an imagelib issue (like I guess the data:// -> Blob suggestion), I'd ask Mike Conley but won't needinfo him here until we determine whether or not the root cause is in imagelib.

I think the regressor is probably something along the line of bug 1135933, which isn't in imagelib - but before that set as desktop background wouldn't have worked in e10s at all - well, not since CPOWs broke / went away, if then.

(In reply to Timothy Nikkel (:tnikkel) from comment #5)

(In reply to :Gijs (he/him) from comment #3)

Never mind, looks like this is likely to have to do with the fact that the e10s implementation here uses a data URI at https://searchfox.org/mozilla-central/rev/250f5cc9fb8bdcbb6b23d2a06acfd48addb2f99b/browser/actors/ContextMenuChild.jsm#248, which fails for an image this large.

I think we probably need to find some other way of passing this information to the parent... We can use a blob URI, but frankly, even painting this whole thing to a canvas to get a blob URI out of it seems dumb. Leaving ni to ask a different question: what better options have we got?

Not really my area of expertise, but even just sending the compressed image source file would be better.

I'm not aware of any IPC primitives that let us do this in the frontend JS / context menu code... are there some and am I just not aware of them, or would we need to invent a way to pass an image this way?

Comment 9

[Mike Conley (:mconley) (:⚙️) (PTO: May 24 - June 2nd)]

(In reply to :Gijs (he/him) from comment #8)

I'm not aware of any IPC primitives that let us do this in the frontend JS / context menu code... are there some and am I just not aware of them, or would we need to invent a way to pass an image this way?

I very recently discovered that we do appear to have a primitive like this - apparently, ImageData can be sent over IPC? We do this here:

https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/dom/canvas/test/test_drawSnapshot.html#21

which via JSWindowActors, sends a message to the parent, which calls into:

https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/testing/specialpowers/content/SpecialPowersParent.jsm#1080-1096

which takes a screenshot, draws it into a canvas, and then extracts the ImageData, and then sends it down to the child.

The child is then able to draw it into a canvas: https://searchfox.org/mozilla-central/rev/01d1011ca4a460f751da030d455d35c267c3e210/dom/canvas/test/test_drawSnapshot.html#22

or could presumably use createImageBitmap to turn it into a bitmap if that's necessary.

Comment 10

[Timothy Nikkel (:tnikkel)]

(In reply to Andrew Overholt [:overholt] from comment #7)

(In reply to :Gijs (he/him) from comment #2)

The regression range and bugs listed as regressors actually suggests this is an imglib issue. Was there some other reason that made you move this out of imglib?

I am curious about the above, too. If it's indeed an imagelib issue then I think we have our winner for who could work on it.
If it's not an imagelib issue (like I guess the data:// -> Blob suggestion), I'd ask Mike Conley but won't needinfo him here until we determine whether or not the root cause is in imagelib.

The regression range is six months long from that comment. And the bugs listed just happen to include the word png in their commit and fall in that six month window. So the regression window given there provides us no useful info to go on.

Comment 11

[abodenlosz]

I was able to reproduce the issue on Firefox 74.0b8 under Windows 10 ARM x64 with image from about:logo.

Comment 12

[:Gijs (he/him)]

(In reply to abodenlosz from comment #11)

I was able to reproduce the issue on Firefox 74.0b8 under Windows 10 ARM x64 with image from about:logo.

The about:logo image is not that big? Can you clarify?

Comment 13

[abodenlosz]

(In reply to :Gijs (he/him) from comment #12)

The about:logo image is not that big? Can you clarify?
The image from about:logo cannot be downloaded via context menu.

Comment 14

[:Gijs (he/him)]

(In reply to abodenlosz from comment #1)

The image from about:logo cannot be downloaded via context menu.

Yeah, that's a different issue. You could file a separate issue, but it's basically the same kind of issue as bug 1387914 so I'd close it as wontfix. It's caused by security restrictions and loosening those just to allow these specific cases has bad risk/reward and cost/benefit balances. The about:logo case is especially unimportant because you can just "save page as" to save only the image - all about:logo is is a pointer to the image.

Comment 15

[Clara Guerrero]

Hi I was able to reproduce the issue on release 73.0.1 (64-bit), beta 74.0b9 (64-bit) and nightly 75.0a1 (2020-03-03) (64-bit) on Windows 10 pro
with the provided link https://eoimages.gsfc.nasa.gov/images/imagerecords/73000/73751/world.topo.bathy.200407.3x21600x10800.png.
I'm updating the flags accordingly. Best, Clara