Ubuntu
firefox package

Saving a web page from Tom's Hardware forums fails

Bug #1820514 reported by Jani Uusitalo
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Fix Released
Medium
firefox (Ubuntu)
Confirmed
Medium
Unassigned

Bug Description

== Steps to reproduce ==
1. Open https://forums.tomshardware.com/threads/what-is-the-actually-difference-between-udimm-and-dimm.1575984/
2. Right-click and select 'Save page as'. Point the dialog to your directory of choice.

== What I expect to happen ==
For the directory to contain a downloaded copy of the page.

== What happens ==
There's no copy of the page in the target directory. Going to about:downloads reveals that the download has failed, without further details.

== Other info ==
I'm reporting this from up-to-date Disco, albeit with a 4.* kernel (as gdm currently fails to start in my VirtualBox VM with 5.*) but it's equally reproducible in Bionic.

Any page under https://forums.tomshardware.com/ seems to trigger this, but I have yet to come across other sites that do. Pages can even be saved from the main Tom's Hardware domain (https://www.tomshardware.com/) without issues.

ProblemType: Bug
DistroRelease: Ubuntu 19.04
Package: firefox 66.0+build3-0ubuntu1
ProcVersionSignature: Ubuntu 4.19.0-9.10-generic 4.19.8
Uname: Linux 4.19.0-9-generic x86_64
AddonCompatCheckDisabled: False
ApportVersion: 2.20.10-0ubuntu23
Architecture: amd64
AudioDevicesInUse:
 USER PID ACCESS COMMAND
 /dev/snd/controlC0: jani 1324 F.... pulseaudio
BuildID: 20190315094614
Channel: Unavailable
CurrentDesktop: ubuntu:GNOME
Date: Sun Mar 17 15:37:04 2019
Extensions: extensions.sqlite corrupt or missing
ForcedLayersAccel: False
IfupdownConfig:
 # interfaces(5) file used by ifup(8) and ifdown(8)
 auto lo
 iface lo inet loopback
IncompatibleExtensions: Unavailable (corrupt or non-existant compatibility.ini or extensions.sqlite)
InstallationDate: Installed on 2018-09-30 (167 days ago)
InstallationMedia: Ubuntu 18.10 "Cosmic Cuttlefish" - Beta amd64 (20180930)
IpRoute:
 default via 10.0.2.2 dev enp0s3 proto dhcp metric 100
 10.0.2.0/24 dev enp0s3 proto kernel scope link src 10.0.2.15 metric 100
 169.254.0.0/16 dev enp0s3 scope link metric 1000
Locales: extensions.sqlite corrupt or missing
PrefSources: prefs.js
ProcEnviron:
 TERM=xterm-256color
 PATH=(custom, no user)
 XDG_RUNTIME_DIR=<set>
 LANG=fi_FI.UTF-8
 SHELL=/bin/bash
Profiles: Profile0 (Default) - LastVersion=66.0/20190315094614 (In use)
RunningIncompatibleAddons: False
SourcePackage: firefox
Themes: extensions.sqlite corrupt or missing
UpgradeStatus: No upgrade log present (probably fresh install)
dmi.bios.date: 12/01/2006
dmi.bios.vendor: innotek GmbH
dmi.bios.version: VirtualBox
dmi.board.name: VirtualBox
dmi.board.vendor: Oracle Corporation
dmi.board.version: 1.2
dmi.chassis.type: 1
dmi.chassis.vendor: Oracle Corporation
dmi.modalias: dmi:bvninnotekGmbH:bvrVirtualBox:bd12/01/2006:svninnotekGmbH:pnVirtualBox:pvr1.2:rvnOracleCorporation:rnVirtualBox:rvr1.2:cvnOracleCorporation:ct1:cvr:
dmi.product.family: Virtual Machine
dmi.product.name: VirtualBox
dmi.product.version: 1.2
dmi.sys.vendor: innotek GmbH

Revision history for this message
Jani Uusitalo (uusijani) wrote :
Revision history for this message
Olivier Tilloy (osomon) wrote :

I can successfully save the page you linked to in the description.
That's on disco, fully up-to-date, on real hardware.

Does saving the page with the Ctrl+S keyboard shortcut yield the same problem?

Revision history for this message
Olivier Tilloy (osomon) wrote :

Also, can you try running firefox in safe mode and see if that makes a difference?

    firefox -safe-mode

Changed in firefox (Ubuntu):
status: New → Incomplete
Revision history for this message
Jani Uusitalo (uusijani) wrote :

Tested all combinations of both (Ctrl-s/context menu in safe mode and normal mode), all with the same results. Could this be locale-dependent? The UI is in English in safe mode though, so I'm guessing safe mode should be locale-independent, as I'm on fi_FI.UTF-8 otherwise.

And just to clarify: my testing of Bionic was on real hardware as well, and I ran these latest tests on Cosmic on another computer (real hardware).

Revision history for this message
Olivier Tilloy (osomon) wrote :

This might be related to https://bugzilla.mozilla.org/show_bug.cgi?id=964213, although that bug is old and without recent activity.

I am now able to reliably observe the problem in a disco VM. When I open the downloads popup and click on the retry icon, saving the page works. Jani, can you confirm this is the case for you too?

Would you mind filing a new bug upstream describing the issue, and link to it here? https://bugzilla.mozilla.org/enter_bug.cgi#h=dupes%7CFirefox

Thanks!

Changed in firefox (Ubuntu):
status: Incomplete → Confirmed
importance: Undecided → Medium
Revision history for this message
In , Jani Uusitalo (uusijani) wrote :

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.121 Safari/537.36

Steps to reproduce:

1. Open https://forums.tomshardware.com/threads/what-is-the-actually-difference-between-udimm-and-dimm.1575984/
2. Right-click and select 'Save page as'. Point the dialog to your directory of choice.

Actual results:

There's no copy of the page in the target directory. Going to about:downloads reveals that the download has failed, without further details.

Expected results:

For the directory to contain a downloaded copy of the page.

Revision history for this message
In , Jani Uusitalo (uusijani) wrote :

(My) original report on Launchpad: https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1820514

Any page under https://forums.tomshardware.com/ seems to trigger this, but I have yet to come across other sites that do. Pages can even be saved from the main Tom's Hardware domain (https://www.tomshardware.com/) without issues.

A workaround discovered by Olivier Tilloy: clicking retry in the download manager completes the save.

Revision history for this message
Jani Uusitalo (uusijani) wrote :

Yep, retrying does work.

I've now reported this to Mozilla and linked to it. Thanks, Olivier!

Bug Watch Updater (bug-watch-updater)
Changed in firefox:
importance: Unknown → Medium
status: Unknown → New
Revision history for this message
In , Paul-boiciuc (paul-boiciuc) wrote :

Hi reporter,

Thank you for taking the time to add this report.
I was able to reproduce the behavior that you have described on all the latest Firefox versions across OS-es ( Windows 10, Ubuntu 18.04 and Mac OS 10.14).
I'm going to add this Downloads Panel component for an advised input. If this is not the proper component, please feel free to change it to a more appropriate one.

Revision history for this message
In , Felipc (felipc) wrote :

Paul_B, can you do a quick check to see if this is a regression?

Revision history for this message
In , Paul-boiciuc (paul-boiciuc) wrote :

I tried running mozregression tool but I could not reach a regression range.
The generated builds for bisection revealed the same issue in 09/2016 / not working build on 06/2015 and a different issue on the last build 11/2014 when the page is saved but with 0 bytes.

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

Saving only the HTML works, "web page, complete" doesn't.

There's some issue with nsIWebBrowserPersist here. There's a `<script>` tag on the page that has a src of `//?loc=https://forums.tomshardware.com/threads/what-is-the-actually-difference-between-udimm-and-dimm.1575984/` . That's a protocol-relative URL, but you can't do that without a hostname, so creating a URL fails, which throws an NS_ERROR_MALFORMED_URI which breaks the DOM walk at https://dxr.mozilla.org/mozilla-central/rev/7f816aa10a2053973c4e6977c5d6f6bf15f38820/dom/webbrowserpersist/WebBrowserPersistLocalDocument.cpp#1101-1104 . Tom's hardware can fix this by making the URL not broken, but obviously webbrowserpersist code shouldn't fail on this.

Moving to where the source code docs say bugs in this component should go.

Bug Watch Updater (bug-watch-updater)
Changed in firefox:
status: New → Confirmed
Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

Gijs, thank you for diagnosing this!

I agree, webbrowserpersist should not treat NS_NewURI failures as fatal for anything other than dealing with that URI.

We could use a general "not Boris" owner for webbrowserpersist. Hsin-Yi, do you have anyone who could do that?

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

huh, how did webbrowserpersist end up to DOM.

Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

Probably because it's doing a DOM walk and should generally be kept up to date with DOM changes...

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

https://searchfox.org/mozilla-central/rev/44a212460990ffffecf50a8e972d3cbde2e7216b/dom/webbrowserpersist/moz.build#7-9 may help in figuring out how this happened. That said, I don't know of anyone actively working on embedding who would own this, and if anything that seems like a poorer fit in terms of what the code is doing...

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

Kiitos Jani bugiraportista.
Tutkailen ja korjailen.

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

Looks like PersistNodeFixup::FixupAnchor already just skips broken urls.

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

Created attachment 9055467
Bug 1536530, let save-page-as work even if there are invalid urls, r=gijs

Revision history for this message
In , Pulsebot (pulsebot) wrote :

Pushed by <email address hidden>:
https://hg.mozilla.org/integration/autoland/rev/d2522fbc3ab9
let save-page-as work even if there are invalid urls, r=Gijs

Revision history for this message
In , Aciure (aciure) wrote :

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

Revision history for this message
In , Htsai (htsai) wrote :

Sorry for being late here. I was off the last two days. Thanks Gijs, Boris and Olli.

Revision history for this message
In , Pascalc (pascalc) wrote :

Olli, do you think that this would be a valuable and safe uplift to beta or should we just let it ride the trains to 68? Thanks

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

Given that we've had this bug at least from FF42 (bug 1101100), it is a bit hard to argue strongly why this should be taken to beta.
But, should be relatively safe.

Revision history for this message
In , Pascalc (pascalc) wrote :

(In reply to Olli Pettay [:smaug] from comment #17)

> Given that we've had this bug at least from FF42 (bug 1101100), it is a bit hard to argue strongly why this should be taken to beta.
> But, should be relatively safe.

The regressing bug wasn't mentioned, so yes, with this additional context it is clearly a wontfix for 67, thanks!

Revision history for this message
In , R-bugs-h (r-bugs-h) wrote :

I didn't say bug 1101100 regressed this. The code has been there at least from that bug, perhaps longer.

Bug Watch Updater (bug-watch-updater)
Changed in firefox:
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.