1207168 - [Page Info] Save As (download) doesn't work when two or more media files are selected at the same time

Status: VERIFIED FIXED

(Firefox :: Page Info Window, defect)

Description

[magicp]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20150921073455

Steps to reproduce:

1. Run Firefox 42.0b1 and later
2. Open about:home
3. View Page Info
4. Select "Media" tab
5. Click "Select All" button
6. Click "Save As" button
7. Select a folder to save the images (e.g. downloads folder)


Actual results:

Save As (download) doesn't work when two or more media files are selected at the same time in the Page Info.


Expected results:

The images are saved to selected folder.

Comment 1

[magicp]

Attachment: Screenshot-Browser-Console.png

Errors in Browser Console

Comment 2

[arni2033]

Reproduced on (Win7_64, Nightly 44, 32bit, ID 20151003030225). Don't know if there's a dupe though.

Comment 3

[magicp]

If this bug was fixed, another bug may occur in Firefox 42 and later version. Because Firefox 41 has a bug with the same steps.

See Also: Bug 1211243

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=04144901baf6&tochange=b32f21db299c

Regressed by: b32f21db299c	Jimmy Wang — Bug 1040947 - Opening page info from a remote tab is sluggish. r=florian

Comment 6

[Loic]

Any update?
Jimmy, as your bug 1040947 is the cause of this regression and if you doesn't work anymore on Page Info, could you needinfo someone in charge of this component, please.

Comment 7

[Jimmy Wang (:jimicy) - works on e10s stuff]

Hi Loic,
I have needinfo'ed florian.
I can also take a look and work on this Friday after most of my exams if florian hasn't already worked on it.

Thanks

Comment 8

[Sandip Dedhia]

is this bug fixed in Firefox 43.0.1 ??? has any one tested ??

Comment 9

[arni2033]

(In reply to Sandip Dedhia from comment #8)
> is this bug fixed in Firefox 43.0.1 ??? has any one tested ??
Yes we did, and no, it's not fixed.
You can usually tell that by "Tracking Flags". It says "status-firefox43: affected"

Comment 10

[Masatoshi Kimura [:emk]]

Indeed saveMedia() will pass aInitiatingDocument == null if multiple media files are selected:
https://mxr.mozilla.org/mozilla-central/source/browser/base/content/pageinfo/pageInfo.js?rev=f4ce7427347e&mark=750-751#750

Comment 11

[Masatoshi Kimura [:emk]]

aIsContentWindowPrivate is 13th argument, but the caller in pageInfo.js passes it in 11th argument.

Comment 12

[Masatoshi Kimura [:emk]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac0e0d4be50b

Comment 13

[Masatoshi Kimura [:emk]]

(In reply to magicp from comment #3)
> If this bug was fixed, another bug may occur in Firefox 42 and later
> version. Because Firefox 41 has a bug with the same steps.

I could also not reproduce bug 1211243 with the patched build. Alice0775, magicp, could you verify with either of the following builds?
Win32:
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-e68519e13b09becee2bb7493177bc43a9affebac/try-win32/firefox-46.0a1.en-US.win32.installer.exe
Win64:
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-e68519e13b09becee2bb7493177bc43a9affebac/try-win64/firefox-46.0a1.en-US.win64.installer.exe

Comment 14

[Alice0775 White]

I can still reproduce bug 1211243 on the try build.
https://hg.mozilla.org/try/rev/e68519e13b09becee2bb7493177bc43a9affebac
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 ID:20160118141314

Comment 15

[Loic]

(In reply to Masatoshi Kimura [:emk] from comment #13)
> (In reply to magicp from comment #3)
> > If this bug was fixed, another bug may occur in Firefox 42 and later
> > version. Because Firefox 41 has a bug with the same steps.
> 
> I could also not reproduce bug 1211243 with the patched build. Alice0775,
> magicp, could you verify with either of the following builds?
> Win32:
> https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-
> e68519e13b09becee2bb7493177bc43a9affebac/try-win32/firefox-46.0a1.en-US.
> win32.installer.exe
> Win64:
> https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-
> e68519e13b09becee2bb7493177bc43a9affebac/try-win64/firefox-46.0a1.en-US.
> win64.installer.exe


Fixed for me with both 32b/64b builds.

Comment 16

[magicp]

Attachment: reproduce-in-try-builds.png

Me too. Please refer to attached image.

Comment 17

[magicp]

(In reply to magicp from comment #16)
> Me too. Please refer to attached image.
  ^ I can still reproduce in Win32/64

Comment 18

[Masatoshi Kimura [:emk]]

Attachment: Fix internalSave() arguments in pageInfo.js

Comment 19

[Florian Quèze [:florian]]

Comment on attachment 8709911 [details] [diff] [review]
Fix internalSave() arguments in pageInfo.js

Review of attachment 8709911 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this, and sorry for missing it during the review of bug 1040947. I wish this was covered by tests.

Comment 20

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0346772141fb5cf69442a6bc0a3e47e0bee05da
Bug 1207168 - Fix internalSave() arguments in pageInfo.js. r=florian

Comment 21

[Carsten Book [:Tomcat]]

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

Comment 22

[Loic]

As it's a very old feature broken recently, could we have that fixed for 45 ESR?

Comment 23

[Masatoshi Kimura [:emk]]

Comment on attachment 8709911 [details] [diff] [review]
Fix internalSave() arguments in pageInfo.js

Approval Request Comment
[Feature/regressing bug #]: bug 1040947
[User impact if declined]: Save as from page info may fail and it will frustrate user.
[Describe test coverage new/current, TreeHerder]: Baked on m-c and manually tested
[Risks and why]: Very low, only fix the wrong function arguments. But please approve this only if bug 1211243 is also approved. Otherwise this may cause dataloss.
[String/UUID change made/needed]: none

Comment 24

[Masatoshi Kimura [:emk]]

Comment on attachment 8709911 [details] [diff] [review]
Fix internalSave() arguments in pageInfo.js

Sorry, this is already in 46 (currently aurora).

Approval Request Comment: See the above comment.

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8709911 [details] [diff] [review]
Fix internalSave() arguments in pageInfo.js

Fix a regression, taking it.
Should be in 45 beta 2.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/10994f5a9532

Comment 27

[magicp]

In case of selecting two or more data URLs which are same type (e.g. Background), downloading files are overwritten to one file.

Comment 28

[Masatoshi Kimura [:emk]]

Oops, probably because uniqueFile() will not work if the file does not already exist on the disk.
But it should prevent Download Manager from removing existing read-only files, at least.

Comment 29

[Kazi Nuzhat Tasnem]

I have reproduced this bug with Firefox aurora 42.0a2 (build id:20150921073455)on
windows 7(64 bit)

I have verified as fixed this bug with Firefox beta 45.0b3(build id:20160204142810)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0

[testday-20160205]

Comment 30

[Nazir Ahmed Sabbir [:NaSb]]

Successfully Reproduced this bug with Firefox Nightly 44.0a1 (2015-09-22) ; (Build ID: 20150922030204) on Linux, 64 Bit with the instructions from comment 0

This Bug is now verified as fixed on Latest Firefox Beta 45.0b7 (Build ID: 20160218171844)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0 and

Latest Firefox Developer Edition 46.0a2 (2016-02-23) ; (Build ID: 20160223004110)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0

Comment 31

[Rezaul Huque Nayeem]

Reproduced the bug in firefox nightly 44.0a1 (2015-09-22) with windows 10 (64 bit)

Verified as fixed with latest firefox beta 45.0b9 (Build ID: 20160223142613)

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

and latest firefox aurora 46.0a2 (Build ID: 20160303004038)

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

Comment 32

[Tummala Dhanvi [:c0mrad3][UTC+5.30]]

Reproduced the bug in Firefox stable 44.0.2 in Debian 8 GNU/Linux 64 bit

Verified as fixed in Firefox ESR 45 in Debian 8 GNU/Linux 64 bit