1679325 - "Save Video As..." prompts to save with file name based on sequential ID instead of original resource name.

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Eavesdown]

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

Steps to reproduce:

Instantiate new, clean profile for testing. Navigate to a page with HTML5 video resources, or directly to a video resource (
e.g., mp4 or webm), such as: https://giant.gfycat.com/VagueSingleGermanwirehairedpointer.webm. Right click the video and select 'Save Video As...' from the context menu.

Actual results:

File save prompt dialog opens offering to save the file with a sequential number (e.g., 10.webm if the browser has just recently been opened). Number appears to increment with each media resource accessed/played by the browser since the last time it was fully closed and re-opened.

Expected results:

File save prompt dialog opens offering to save the file with the original resource name of VagueSingleGermanwirehairedpointer.webm.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 2

[Andrew Overholt [:overholt]]

Thanks for the report, Eavesdown! If you have time, can you run mozregression to see when this regressed (some time after Firefox 83)? https://mozilla.github.io/mozregression/

Tim, could bug 1641270 have caused this regression (it works for me in 83 release but not Nightly)?

Comment 3

[Tim Huang[:timhuang]]

Yes, Bug 1641270 has caused this issue. I will fix this.

Comment 4

[Andrew Overholt [:overholt]]

Thanks, Tim!

Comment 5

[Tim Huang[:timhuang]]

Attachment: Bug 1679325 - Part 1: Making contentAreaUtils.saveURL to accept cookieJarSettings. r?smaug!

This patch makes the contentAreaUtils.saveURL to be aware of the
cookieJarSettings, and updates all callers.

This also updates the documentation of the persistArgs
'cookieJarSettings' for internalPersist().

Comment 6

[Tim Huang[:timhuang]]

Attachment: Bug 1679325 - Part 2: Add a test to ensure the saveVideoAs is correctly partitioned and the default file name is correct. r?smaug!,dimi!

Depends on D98455

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Is this Nightly and early beta issue only? Or do we need to land the fix to beta too?

Comment 8

[Eavesdown]

I should have mentioned in my report that I was experiencing the issue on the latest beta version of Dev Edition. I can confirm that it's still present in 84.0b7, as well. I'm not entirely familiar with the release structure of Firefox at this point, so I don't know whether Dev Edition runs "early" betas at any point in its release cycle, but my understanding was that it's on the same track as the normal betas now (i.e., not really an "Aurora" channel anymore). If that understanding is accurate, then the fix will be needed in the standard beta channels, as well.

Comment 9

[Tim Huang[:timhuang]]

I think we have to land the fix to beta too because there is a bug in my patch in Bug 1641270 which causes this issue. I will request the uplift once this is resolved.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/315bab44858b
Part 1: Making contentAreaUtils.saveURL to accept cookieJarSettings. r=smaug
https://hg.mozilla.org/integration/autoland/rev/230f99ad861d
Part 2: Add a test to ensure the saveVideoAs is correctly partitioned and the default file name is correct. r=smaug,dimi

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/315bab44858b
https://hg.mozilla.org/mozilla-central/rev/230f99ad861d

Comment 12

[Tim Huang[:timhuang]]

Comment on attachment 9190792 [details]
Bug 1679325 - Part 1: Making contentAreaUtils.saveURL to accept cookieJarSettings. r?smaug!

Beta/Release Uplift Approval Request

• User impact if declined: The default file name of 'Save Video As' will be incorrect.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch itself is not complex. It only fixes several places to use the proper parameter. And we have the test. So, I believe this is a low risk.
• String changes made/needed: Nope

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9190792 [details]
Bug 1679325 - Part 1: Making contentAreaUtils.saveURL to accept cookieJarSettings. r?smaug!

Ouch. Approved for 84.0rc1. Thanks for including a test.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/11f96ee5c8d9
https://hg.mozilla.org/releases/mozilla-beta/rev/ed9b072ec4e0

Comment 16

[Eavesdown]

I can verify that this issue has been successfully resolved for my configuration as of the 85.0b1 release.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/dc74addeeb70
adjust saveURL arguments after bug 1641270. r=lasana