1405199 - in Version 56, *.URL shortcuts open cached version of webpage and show local path to shortcut instead of live webpage and URL

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Richard]

User Agent: Mozilla/5.0 (connectpro; Windows NT 6.1; WOW64; Trident/7.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; Creative AutoUpdate v1.40.01; rv:11.0) like Gecko

Steps to reproduce:

Firefox updated on 200+ computers and all have this issue. 

We click on existing or new created shortcut on the desktop or anywhere in the file system on Windows 10 with FF as default. 


Actual results:

FF opens with a cached copy of the webpage and shows the local path to the shortcut *.url file.


Expected results:

FF show have opened the live webpage and showed the URL of the site.

Comment 1

[Richard]

FF v55 does not have this issue. If we uninstall 56 and install 55 on the same computer FF open *.url files correctly. 

Furthermore each time v56 opens a *.url file and show a cached copy there is some FF task that get launched on the background which will not close correctly when FF is closed or when the user logs off Windows.

Should the user return to the same computer FF will open indicating Safe mode of refresh FF. If however the user roams to another computer FF will no longer launch until any FF process is killed until FF launches indicating Safe mode of refresh FF.

Comment 2

[jscher2000]

Related SuMo threads: 

(1) https://support.mozilla.org/questions/1177925 (Windows 10/WOW64)

Seems caused by:

[HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\Explorer\FileExts\.url\UserChoice]
"ProgId"="Applications\\firefox.exe"

instead of:

[HKEY_CURRENT_USER\SOFTWARE\Microsoft\Windows\CurrentVersion\Explorer\FileExts\.url\UserChoice]
"ProgId"="InternetShortcut"

(2) https://support.mozilla.org/questions/1177509 (Windows 7/Win64 UA, not independently confirmed)

Original poster not responding recently.

Comment 3

[Richard]

Also found this but V55.0.3 and prior have no issue with this very same key. Also that key requires a special hash that's created by Windows from the user name, computer name and app. So with 200+ computers and hundreds of users, plus those keys have custom permissions that don't allow normal deletion it very hard to even remove the two keys nor is is possible to change them with a script/GPO. I have bee posting to that 1177509 and create a new question 1177925 as NPLD.

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Jason - could this be a change to prefer cached content on startup-and-load cases in 56+?   I was simply guessing we might have changed startup behavior to speed startup (avoid pinging pages if we think the cached content is still valid)

Comment 5

[Honza Bambas (:mayhemer)]

Could potentially be caused by bug 1319111.

Comment 6

[Robert Strong (they/them - no direct email)]

The setting of registry entries like this is performed by the installer. Shell integration just detects whether the individual installation of Firefox is the default browser and launches the OS ui for setting as default.

Comment 7

[Honza Bambas (:mayhemer)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #6)
> The setting of registry entries like this is performed by the installer.
> Shell integration just detects whether the individual installation of
> Firefox is the default browser and launches the OS ui for setting as default.

Thanks, that's actually the right component I wanted to move this into!

Comment 8

[Richard]

Just some more details. We silently install the FF updates using Ninite Pro, but even if we run the FF 56.0 installer logged in to a machine via the installer GUI or with the -ms silent switch the outcome is the same. In our case we already have FF set as the default so the installer in GUI mode or FF launching the first time after an upgrade does not ask about being made default nor brings up the Windows Default Apps setting page. 
Thank you for looking into this.

Comment 9

[Molly Howell (she/her) (mostly inactive)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #6)
> The setting of registry entries like this is performed by the installer.

... is correct, but this bug is not an installer bug, this is a command-line handling bug. The installer doesn't touch anything in the Explorer\FileExts key or otherwise do anything related to .url files (I can only reproduce the bug by manually changing the association for .url files), but even if it did, we should be opening .url files passed in from the command line correctly. Simply running the command "firefox.exe <path to a .url file> also gets the broken behavior.

But, if you drag a .url file into an open Firefox window, you get the correct behavior.

So evidently something is wrong with how we are opening .url files from the command line, but not from elsewhere.

mozregression has pointed me to bug 1319111, which is a decent size patch to code that I don't know anything about, so I think I'll just ni? Honza again. ;)

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

I can track this for 56, but it seems unlikely to be fixed in 56. Maybe more realistic to aim for 57. 
Honza, are you able to work on this?

Comment 12

[Honza Bambas (:mayhemer)]

Thanks Matt for STR and regression range!

As this leads to bug 1319111 then yes, I will work on this and soon.

Comment 13

[Honza Bambas (:mayhemer)]

Attachment: v1 (basechannel has to update rpuri on the new channel before changing its orig-uri)

- same update is needed as in case http channel redirects at [1] ; base channel lacked this update
- I'm looking for rpURI on the *existing* loadinfo object taken from the new channel first ; this preserves the rpURI in case when the target URI handler sets it on the new channel, we replace loadinfo on the new channel with a clone of the one given to the original base channel


Apparently, setting LOAD_REPLACE in top of nsBaseChannel::Redirect had to kick me...


[1] https://dxr.mozilla.org/mozilla-central/rev/5eba13f5b3a6ad80decdd8c7b30bff5fa477844f/netwerk/protocol/http/HttpBaseChannel.cpp#3354

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8916628 [details] [diff] [review]
v1 (basechannel has to update rpuri on the new channel before changing its orig-uri)

r=me.  I'm sorry I missed this in review.  :(

Comment 15

[Honza Bambas (:mayhemer)]

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #14)
> Comment on attachment 8916628 [details] [diff] [review]
> v1 (basechannel has to update rpuri on the new channel before changing its
> orig-uri)
> 
> r=me.  I'm sorry I missed this in review.  :(

I think you did not, we mentioned that but I somehow either decided it was not needed or forgot to update.

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/41697bca82fb
Update result principal URI on the new channel when nsBaseChannel redirects. r=bz

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/41697bca82fb

Comment 18

[Richard]

Thank you for working to fix this. What are the chances this fix might make it into v56.0.2 or v57?

Comment 19

[Boris Zbarsky [:bzbarsky]]

The chances of 56.0.2 are pretty low, I think (e.g. there are no plans to have one that I'm aware of).

For 57, Honza, how do you feel about beta uplift?

Comment 20

[Honza Bambas (:mayhemer)]

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #19)
> The chances of 56.0.2 are pretty low, I think (e.g. there are no plans to
> have one that I'm aware of).
> 
> For 57, Honza, how do you feel about beta uplift?

Positively.

Comment 21

[Honza Bambas (:mayhemer)]

Comment on attachment 8916628 [details] [diff] [review]
v1 (basechannel has to update rpuri on the new channel before changing its orig-uri)

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1319111 

[User impact if declined]:
incorrect behavior when loading .URL files directly or opening from shell with non-standard (but not uncommon) registry settings (bad URL shown, incorrect reload behavior) ; potentially could lead to other security sensitive bugs

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Yes, locally as per comment 9

[Needs manual test from QE? If yes, steps to reproduce]: 
See comment 0 and comment 9

[List of other uplifts needed for the feature/fix]:
none

[Is the change risky?]:
hard to assess, but I'd say no

[Why is the change risky/not risky?]:
it's a straightforwards well understood change

[String changes made/needed]:
none

Comment 22

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8916628 [details] [diff] [review]
v1 (basechannel has to update rpuri on the new channel before changing its orig-uri)

Recent regression, beta57+

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/361361c9a042

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Will ride the train with 57.

Comment 26

[Oana Botisan, Desktop QA]

I managed to reproduce the bug on an older version of Nightly from 2017-10-02 using Windows 10x64 by activating Menu Bar - Open File... and selecting an .url file previously made. 

I tested using the same method on latest Nightly and beta 57.0b12, but the bug is not reproducing anymore. 

Is the method I tried valid to confirm the bug fix? Because using other methods (as setting an older version of Nightly as default) I couldn't reproduce the bug. Did I miss something?

Comment 27

[Honza Bambas (:mayhemer)]

I used this to verify (from comment 9):

"Simply running the command "firefox.exe <path to a .url file> also gets the broken behavior."

Comment 28

[Oana Botisan, Desktop QA]

I managed to reproduce the bug on an older version of Nightly (2017-10-02)using the method from comment 9 on Windows 10x64.
I retested on latest Nightly 58 and beta 57.0b12 and it's not reproducing anymore.