Closed Bug 1641270 Opened 4 years ago Closed 4 years ago

Saving already-loaded images from a webpage yields "not an image".

Categories

(Core :: Graphics: ImageLib, defect, P2)

Product:
Core
Component:
Graphics: ImageLib
Version:
Firefox 84
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
84 Branch
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- disabled
firefox77 --- unaffected
firefox78 --- disabled
firefox79 --- disabled
firefox80 --- disabled
firefox81 --- disabled
firefox83 --- disabled
firefox84 --- verified

People

(Reporter: michiel, Assigned: timhuang)

References

(Regression)

Details

(Keywords: regression)

Attachments

(9 files)

Bug 1641270 - Part 1: Expose cookieJarSettings to UI code through Document. r?smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 2: Make WebBrowserPersistDocument aware of CookieJarSettings. r?smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 3: Make nsICookieJarSetting serializable. r?dimi
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 4: Expose the cookieJarSettings to the browser object. r?dimi,smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 5: Send cookieJarSettings across processes for ContextMenu. r?smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 6: Pass CookieJarSettings to nsIWebBrowserPersist.saveURI() and nsIWebBrowserPersist.savePrivacyAwareURI(). r?smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 7: Add setter/getter of nsICookieJarSettings to nsITransferable. r?smaug
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 8: Make the Drag&Drop to use the correct cookieJarSettings to download the url. r?smaug,dimi
47 bytes, text/x-phabricator-request
Details | Review
Bug 1641270 - Part 9: Add a test case to ensure the saving channel will use the correct cookieJarSettings. r?dimi
47 bytes, text/x-phabricator-request
Details | Review

Nightly no longer just "saves images" when right clicking on an image and using "save as", or even drag-and-dropping from a webpage to the desktop.

For example: if I click-drag the already-loaded image from https://www.pixiv.net/en/artworks/81877591 to my desktop, instead of saving that image, it saves an HTML file with the image's filename, with content:

<html>
<head><title>403 Forbidden</title></head>
<body>
<center><h1>403 Forbidden</h1></center>
<hr><center>nginx</center>
</body>
</html>

This suggests FF tries to redownload the image, which it shouldn't: the image was already loaded, FF should not be doing any network requests, instead writing that data it already has.

Right clicking and hitting save as works for me. Dragging creates a link. You said "no longer just", so I assume this used to work. Can you use mozregression to find the offending patch? Thanks!

Flags: needinfo?(pomax)
Assignee: nobody → afarre

And could you please check the value of the 'fission.autostart' pref? I've made some changes recently to how 'Save-As' works in the context of Fission, which might be relevant.

I can reproduce the drag and drop bug on Windows. I get a jpeg file containing html. This is a recent regression.

Keywords: regression, regressionwindow-wanted
Priority: -- → P1

Freddy, mozregression claims bug 1613609 regressed this. Can you take a look?

Flags: needinfo?(fbraun)
Severity: -- → S1

[Tracking Requested - why for this release]:
regression with drag-and-drop of images

tracking-firefox78: --- → ?

I can also reproduce the drag and drop bug on Windows10.
However, I got a different regression window...

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a55d8ca4738f9b51515959de087df55e5d5999d8&tochange=7e53b5398797485e8f326bde1fcb5820afcbc8a2

I'm looking, but due to the regressions, the commit for bug 1613609 that actually stuck had all major changes preffed off.

This seems to be an issue of two parts:
First of all, I was unable to drag any files into my linux desktop or a file manager, which might be a different issue.
So I opened a new Firefox window and used that as a drop target

Then, comparing the reported URL from Pomax with another image load (e.g., https://www.mozilla.org/media/contentcards/img/home-2019/card_1/reads.9b652ec5bff6.png), I noticed that in both cases the image is re-requested, but here's the thing:
pivix.net sends a 403 Forbidden, presumably to disallow hotlinking (cf. https://en.wikipedia.org/wiki/Inline_linking#Controversial_uses_of_inline_linking).
The linked image from mozilla.org does NOT show this behavior and loads just fine.

It seems that this might be related to reloading but also to inheriting (or not inheriting) cookie, referrer and other information.

Pomax, can you please share whether you get this behavior on pivix.net and mozilla.org or just on the former?
baku, could this be related to the push mentioned in comment 7?

Flags: needinfo?(fbraun) → needinfo?(amarchesini)

This seems similar to bug 1640405.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(amarchesini)
Resolution: --- → DUPLICATE

I'll track in the other bug.

tracking-firefox78: ? → ---

Responding in reverse order:

<<Pomax, can you please share whether you get this behavior on pivix.net and mozilla.org or just on the former?>>

Mozilla.org works fine, the problem is Firefox going "I will not save the image I already have, I will perform a new network request", which can go wrong in quite a lot of ways, but most obviously goes wrong with pixiv (which has a 403 in place as anti-hot-linking counter measure).

<<Right clicking and hitting save as works for me. Dragging creates a link>>

Not actually a link, though - it's supposed to still be the same image; it looks like Jim found the change that introduced the problem?

Flags: needinfo?(pomax)

It looks like bug 1640405 got a fix that landed several days ago, but that hasn't fixed the image issue reported here for nightly. As such, this might need to be reopened and addressed as being a different issue rather than closed as duplicate?

Flags: needinfo?(amarchesini)

Yes, I can still reproduce this issue on Nightly79.0a1(20200611093454) windows10.

Status: RESOLVED → REOPENED
status-firefox79: --- → affected
Resolution: DUPLICATE → ---

setting privacy.partition.network_state = false will fix.

Keywords: regressionwindow-wanted
Regressed by: 1639154
Has Regression Range: --- → yes

That pref is only enabled in nightly, updating status for 78.

status-firefox78: affected → disabled
See Also: → 1650609

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: General → ImageLib

Note that this is still happening in 82.0a1 nightly, the only workaround is to set the setting privacy.partition.network_state pref to false, which of course isn't a solution. FF should not be initiating a second network request at all, it should be copying the data from the cache dir, so as to guaranteed save the same data that the user is looking at (especially as consecutive network requests to a specific URL are in no way guaranteed to yield the same data)

Severity: S1 → S3
Priority: P1 → P2

I believe this issue is because we don't use the correct CookieJarSettings when saving the image here. The downloading channel won't have the correct partitionKey, so it won't get the image from the cache.

I will try to fix this. Farre, it seems that you are not currently working on this bug. I hope that you don't mind that I take this bug from you.

Assignee: afarre → tihuang
Status: REOPENED → ASSIGNED

In this patch, we add cookieJarSettings in WebBrowserPersistDocument.
This is needed if we want to use the correct cookieJarSettings when
download the page or URI.

Depends on D95611

In order to send the cookieJarSetting across processes in JS. We need to
make it serializable.

Depends on D95612

This patch makes the ContextMenu to send cookieJarSettings from content
to the parent process via ContextMenuChild/Parent. The cookieJarSettings
will be used in 'Save ... As'.

Depends on D95614

Version: 78 Branch → Firefox 84

I see there's a needinfo on :baku from 5 months ago, is that still relevant?

Blocks: 1659578
Pushed by tihuang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fcba6dba3649 Part 1: Expose cookieJarSettings to UI code through Document. r=smaug,dimi https://hg.mozilla.org/integration/autoland/rev/2663dc1e9a3e Part 2: Make WebBrowserPersistDocument aware of CookieJarSettings. r=smaug https://hg.mozilla.org/integration/autoland/rev/7bef9112f296 Part 3: Make nsICookieJarSetting serializable. r=dimi https://hg.mozilla.org/integration/autoland/rev/55ea7810c160 Part 4: Expose the cookieJarSettings to the browser object. r=dimi https://hg.mozilla.org/integration/autoland/rev/265ae2953416 Part 5: Send cookieJarSettings across processes for ContextMenu. r=smaug https://hg.mozilla.org/integration/autoland/rev/143728b1b1ab Part 6: Pass CookieJarSettings to nsIWebBrowserPersist.saveURI() and nsIWebBrowserPersist.savePrivacyAwareURI(). r=smaug https://hg.mozilla.org/integration/autoland/rev/dfeb879f4131 Part 7: Add setter/getter of nsICookieJarSettings to nsITransferable. r=smaug https://hg.mozilla.org/integration/autoland/rev/5f9c5af66b77 Part 8: Make the Drag&Drop to use the correct cookieJarSettings to download the url. r=smaug,dimi https://hg.mozilla.org/integration/autoland/rev/89a6dab92f1a Part 9: Add a test case to ensure the saving channel will use the correct cookieJarSettings. r=dimi
Flags: needinfo?(tihuang)
Pushed by tihuang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c139182fe0fe Part 1: Expose cookieJarSettings to UI code through Document. r=smaug,dimi https://hg.mozilla.org/integration/autoland/rev/c7d2b51f8489 Part 2: Make WebBrowserPersistDocument aware of CookieJarSettings. r=smaug https://hg.mozilla.org/integration/autoland/rev/33587929b37b Part 3: Make nsICookieJarSetting serializable. r=dimi https://hg.mozilla.org/integration/autoland/rev/71f4bfbfab6a Part 4: Expose the cookieJarSettings to the browser object. r=dimi https://hg.mozilla.org/integration/autoland/rev/8774f126201a Part 5: Send cookieJarSettings across processes for ContextMenu. r=smaug https://hg.mozilla.org/integration/autoland/rev/12c36ebc9fba Part 6: Pass CookieJarSettings to nsIWebBrowserPersist.saveURI() and nsIWebBrowserPersist.savePrivacyAwareURI(). r=smaug https://hg.mozilla.org/integration/autoland/rev/b99be18d8305 Part 7: Add setter/getter of nsICookieJarSettings to nsITransferable. r=smaug https://hg.mozilla.org/integration/autoland/rev/78b7742630fc Part 8: Make the Drag&Drop to use the correct cookieJarSettings to download the url. r=smaug,dimi https://hg.mozilla.org/integration/autoland/rev/e93201b4e908 Part 9: Add a test case to ensure the saving channel will use the correct cookieJarSettings. r=dimi
Flags: qe-verify+
Regressions: 1679489
Regressions: 1680868
Regressions: 1679325

Reproduced the initial issue using old Nightly from 2020-05-27, verified that this is fixed now using Firefox 84.0 on Windows 10 64bit and Windows 8.1 64bit.

Status: RESOLVED → VERIFIED
status-firefox84: fixed → verified
Flags: qe-verify+
Regressions: 1683244
See Also: → 1694676

clearing my needinfo flag.

Flags: needinfo?(amarchesini)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: