Closed Bug 1850954 Opened 1 year ago Closed 1 year ago

Cant change URL.protocol since v117

Categories

(Core :: Networking: HTTP, defect, P1)

Firefox 117
defect

Tracking

()

RESOLVED FIXED
119 Branch
Tracking Status
relnote-firefox --- 117+
firefox-esr102 --- unaffected
firefox-esr115 --- unaffected
firefox117 + fixed
firefox118 + fixed
firefox119 + fixed

People

(Reporter: logansm, Assigned: twisniewski)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [necko-triaged])

Attachments

(1 file)

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

Steps to reproduce:

//Javascript to change the protocol of an URL instance
const url = new URL("https://www.anypage.com?query=something")
url.protocol = "nlp:"
console.log(url.href)

Actual results:

//protocol of the URL object stays on "http" or "https" and does not change.
url.href equals "https://www.anypage.com?query=something"

Expected results:

//this worked before upgrading to Firefox version 117
//protocol of the URL object changes to "nlp:"
url.href equals "nlp://www.anypage.com?query=something"

The new behavior is the correct as defined by the specification, but maybe not web compatible. Did this break your website?

Keywords: regression
Regressed by: 1347459

The Bugbug bot thinks this bug should belong to the 'Core::Networking: HTTP' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Networking: HTTP
Product: Firefox → Core

:twisniewski, since you are the author of the regressor, bug 1347459, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Flags: needinfo?(twisniewski)

(In reply to Tom S [:evilpie] from comment #1)

The new behavior is the correct as defined by the specification, but maybe not web compatible. Did this break your website?

Yes it breaks some features of my website. The custom protocols are needed to launch applications. Since it works for other browsers and also worked for Firefox in previous version aswell, I need to implement temporarily a workaround for Firefox users.

Alright, then since bug 1347459 is causing webcompat issues, I think it would be best to revert it and file a spec bug.

Severity: -- → S1
Flags: needinfo?(twisniewski)
Priority: -- → P1

Chrome also fixed this in https://bugs.chromium.org/p/chromium/issues/detail?id=1416018 (Chrome 118).
See https://chromium-review.googlesource.com/c/chromium/src/+/4744799
Safari hasn't yet fixed the issue, so we were the first to ship this.

The question is whether we want to back this out and/or put it behind a pref until the other browsers ship it too.
Thomas, do you want to take the lead on this?

Blocks: url
Flags: needinfo?(twisniewski)
Whiteboard: [necko-triaged]
Status: UNCONFIRMED → NEW
Ever confirmed: true

I'm working on a backout patch now, but I don't have too much free time to push for a spec update right now. Do you think you or someone else could help out with that?

Flags: needinfo?(twisniewski) → needinfo?(valentin.gosu)

Set release status flags based on info from the regressing bug 1347459

So Google's going to be shipping this same change in ~5 weeks?

Good point, Ryan. If Google is about to ship this, then maybe it's worth thinking about this a bit more before doing any backouts, given that this is the only report we've received about breaking in two months.

Han, are the changes you need to make drastic?

Severity: S1 → S3
Flags: needinfo?(valentin.gosu)
Priority: P1 → --

The change itself is quite simple I guess. Currently I am not sure which is the best way to solve this (fast and secure), I go the ugly way with string replace (regex) or I build a new URL object with something like

//js
new Url("customprotocol://" + oldUrl.host + oldUrl.pathname + oldUrl.search + oldUrl.hash)

which is kinda reinventing the wheel to me, guess url.href is doing this already.

I am not sure how much you guys are affected by the fact, that my personal challange comes with rolling out the hotfix to more than 1k clients and that I am bound to strict release cyclies and release time frames.
But I understand, since its a wave which will hit me anyway sooner or later with Google Chrome, MS Edge (a.k.a mini Chrome), Safari and so on, I need to come up with something.

I left a comment on this standard issue: https://github.com/whatwg/url/issues/674#issuecomment-1701304722

I think it's best to backout the regressor for now in order to avoid webcompat causing issues for our users.
We can probably land it back in nightly with a pref toggle, depending on how the standard discussions go.

Assignee: nobody → twisniewski
Priority: -- → P1

We can take it in the mid-cycle dot release maybe. Might be worth letting this percolate for a bit to see what other reports we get. Given that timing, would it make more sense to go with the pref route rather than backing out?

Valentin and I briefly chatted a little earlier about this, and we've decided to back this out for now while we await a reply from Google. We may re-land this behind a pref soon, but for now I'll land a backout patch here unless the try-run shows any worrying signs: https://treeherder.mozilla.org/jobs?repo=try&revision=38b300afe372cff51f7238f1dc4eee414908d9de

(In reply to Thomas Wisniewski [:twisniewski] from comment #10)

Good point, Ryan. If Google is about to ship this, then maybe it's worth thinking about this a bit more before doing any backouts, given that this is the only report we've received about breaking in two months.

Han, are the changes you need to make drastic?

For the sake of correctness I wanted to mention that Firefox v117 was released on August 29th, so two days before this report. I read the second time "in two months" (here and also in the Webkit bug entry). I'm a little confused about the calculation, are you referring to beta releases?

The bug is marked as tracked for firefox117 (release), tracked for firefox118 (beta) and tracked for firefox119 (nightly). However, the bug still has low severity.

:ghess, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Flags: needinfo?(ghess)
Pushed by twisniewski@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8fcbbb10c2e5 Back out URL protocol spec-compliant restrictions added in bug 1347459 for causing webcompat issues; r=valentin,necko-reviewers,kershaw
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 119 Branch

Chromium 118 goes to Stable Release on Tue, Oct 10, 2023.
According to Chrome devs, they don't intend to revert the change.
The only question is whether we want to uplift the backout to the dot release, or be the first to ship this change and take the webcompat hit (if there is any) for a little over a month.

Flags: needinfo?(ghess)

The patch landed in nightly and beta is affected.
:twisniewski, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(twisniewski)
Duplicate of this bug: 1851565

If we want to get this into next week's 117.0.1 release, this needs Beta and Release approval requests ASAP.

Flags: needinfo?(valentin.gosu)

Comment on attachment 9351097 [details]
Bug 1850954 - Back out URL protocol spec-compliant restrictions added in bug 1347459 for causing webcompat issues; r?valentin

Beta/Release Uplift Approval Request

  • User impact if declined: Potential for sites breaking. Although changing URL protocols isn't that common, it could definitely break behaviour if a website isn't expecting it. We should back out this change, and land it at a later time (after Chrome does so also, and with a pref).
  • 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): Reverts to previous behaviour.
  • String changes made/needed:
  • Is Android affected?: Yes
Flags: needinfo?(valentin.gosu)
Attachment #9351097 - Flags: approval-mozilla-release?
Attachment #9351097 - Flags: approval-mozilla-beta?

Comment on attachment 9351097 [details]
Bug 1850954 - Back out URL protocol spec-compliant restrictions added in bug 1347459 for causing webcompat issues; r?valentin

Approved for 118.0b7, thanks.

Attachment #9351097 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9351097 [details]
Bug 1850954 - Back out URL protocol spec-compliant restrictions added in bug 1347459 for causing webcompat issues; r?valentin

Approved for 117.0.1.

Flags: needinfo?(twisniewski)
Attachment #9351097 - Flags: approval-mozilla-release? → approval-mozilla-release+

Added to the 117.0.1 release notes:

Temporarily reverted an intentional behavior change preventing Javascript from changing URL.protocol (bug 1850954). NOTE: This change is expected to ship in a later Firefox release alongside other web browsers and sites are encouraged to find alternate ways to change the protocol if needed.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: