Remove UserAgentOverrides.jsm
Categories
(Core :: Networking, enhancement, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: valentin, Assigned: valentin)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-triaged])
Attachments
(4 files)
UserAgentOverrides should be rewritten as a C++ service for performance and correctness reasons.
Assignee | ||
Comment 1•5 years ago
|
||
So, from what I can tell, following bug 1505722, UserAgentOverrides.jsm isn't even needed anymore. Bug 1511281 can be fixed using nsIDocshell.customUserAgent, meaning the need for UserAgentOverrides.jsm and UserAgentUpdates.jsm is pretty much gone. Dennis, from what I can tell the webcompat addon is capable enough to replace what we already have here. Can you think of any reason not to remove this code?
Assignee | ||
Comment 2•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6af91996de0b22031582953edd084e73c3b848c
Comment 3•5 years ago
|
||
Hey Valentin, thanks for the ping! You are not wrong, the webcompat addon is able to do what we did with UserAgentOverrides.jsm, and although we landed the existing overrides in bug 1512907 yesterday, we are unfortunately not yet ready to remove the old code. One important piece missing on our side is to verify that we can push updates of that addon to release users on both desktop *and* mobile. Pushing UA overrides to the release population is very important, and we have used that multiple times to fix completely-broken sites. We are working on this, and will be testing the rollout very soon. We're pretty sure this work will be done in Q1. Unless we have a very urgent reason to remove UserAgentOverrides.jsm right now, it's probably better (for us) to just leave the old code in place for now. I will ping back as soon as we have verified that our plans work out!
Assignee | ||
Comment 4•5 years ago
|
||
(In reply to Dennis Schubert [:denschub] from comment #3) > We are working on this, and will be testing the rollout very soon. We're > pretty sure this work will be done in Q1. Unless we have a very urgent > reason to remove UserAgentOverrides.jsm right now, it's probably better (for > us) to just leave the old code in place for now. I will ping back as soon as > we have verified that our plans work out! That's great. I'll have the patches to remove reviewed and ready. The only rush to remove the code would be bug 1513582 comment 0 ; bug 1443429 comment 23 This bug was initially to rewrite the code in C++, but I'd rather to not do all that work just to remove it a few weeks later. :) Could you make the "push updates of that addon to release users" bug block this one? Thanks!
Assignee | ||
Comment 5•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec4b36de13feada061ea38860200df6697cba95a
Assignee | ||
Comment 6•5 years ago
|
||
This patch removes the use fo UserAgentOverrides from browser.js The UA change when in desktop mode now uses nsIDocShell.customUserAgent, while the feature landed in bug 838332 that is only performed for t.co URLs is removed, as it landed 4 years ago and was limited to Nightly.
Assignee | ||
Comment 7•5 years ago
|
||
Depends on D14749
Assignee | ||
Comment 8•5 years ago
|
||
Also removes the UA cache attached to nsILoadGroup and nsIRequestContext and the "http-on-useragent-request" observer notification. If overriding the user agent is needed "http-on-modify-request" is equally usable (but should be used rarely, for performance reasons). A better way is using nsIDocShell.customUserAgent. Depends on D14750
Assignee | ||
Comment 9•5 years ago
|
||
Depends on D14751
Assignee | ||
Comment 10•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f90d39b730ed916dbc67a07d93bd9f21ff406d74
Assignee | ||
Comment 11•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=afcfc368608d6e31e2e9e2a1a894c387f8d08889
Comment 12•5 years ago
|
||
> Could you make the "push updates of that addon to release users" bug block this one? Thanks!
Absolutely! I filed a meta bug. Right now, we track a lot of this work outside of Bugzilla, but will make sure that the bug gets updated, and closed, when there is progress.
Comment 13•5 years ago
|
||
There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:valentin, could you have a look please?
Assignee | ||
Comment 14•5 years ago
|
||
I was waiting on bug 1515005 before I can land the patches.
Adam, I see that bug 1527352 is verified. Can we remove UserAgentOverrides.jsm now?
Updated•5 years ago
|
Comment 15•5 years ago
|
||
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0e4cababccc7 Remove UAOverride use from mobile/.../browser.js r=snorp https://hg.mozilla.org/integration/autoland/rev/5f8b5f72a4ea Remove UAOverridesBootstrapper r=michal https://hg.mozilla.org/integration/autoland/rev/1f2bbbe0f781 Remove UserAgentOverrides.jsm and UserAgentUpdates.jsm r=michal https://hg.mozilla.org/integration/autoland/rev/f155c449e516 Remove ua-update.json.in from android build r=snorp
Comment 16•5 years ago
|
||
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=266581728&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=266582414&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/8c3cf0c548977009df6f6c690c7f7335b032626f
[task 2019-09-13T18:25:18.357Z] 18:25:18 INFO - Killing every process called emulator64-arm
[task 2019-09-13T18:25:18.364Z] 18:25:18 INFO - Killing pid 177.
[task 2019-09-13T18:25:18.367Z] 18:25:18 INFO - [mozharness: 2019-09-13 18:25:18.364740Z] Finished run-tests step (failed)
[task 2019-09-13T18:25:18.367Z] 18:25:18 FATAL - Uncaught exception: Traceback (most recent call last):
[task 2019-09-13T18:25:18.368Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/src/testing/mozharness/mozharness/base/script.py", line 2101, in run
[task 2019-09-13T18:25:18.369Z] 18:25:18 FATAL - self.run_action(action)
[task 2019-09-13T18:25:18.369Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/src/testing/mozharness/mozharness/base/script.py", line 2040, in run_action
[task 2019-09-13T18:25:18.369Z] 18:25:18 FATAL - self._possibly_run_method(method_name, error_if_missing=True)
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/src/testing/mozharness/mozharness/base/script.py", line 1995, in _possibly_run_method
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - return getattr(self, method_name)()
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/src/testing/mozharness/scripts/android_emulator_pgo.py", line 229, in run_tests
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - env=env,
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 462, in init
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - self.start_binary(self.startup_timeout)
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 491, in start_binary
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - reraise(IOError, msg.format(timeout), tb)
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 482, in start_binary
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - self.raise_for_port(timeout=timeout)
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - File "/builds/worker/workspace/build/venv/lib/python2.7/site-packages/marionette_driver/marionette.py", line 569, in raise_for_port
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - self.host, self.port))
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - IOError: Process killed after 1000s because no connection to Marionette server could be established. Check gecko.log for errors
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - Running post_fatal callback...
[task 2019-09-13T18:25:18.370Z] 18:25:18 FATAL - Exiting -1
[task 2019-09-13T18:04:38.933Z] 18:04:38 INFO - Console message: [JavaScript Error: "RemoteWebProgress failed to call onProgressChange: [Exception... "JavaScript component does not have a method named: "onProgressChange"'JavaScript component does not have a method named: "onProgressChange"' when calling method: [nsIWebProgressListener::onProgressChange]" nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)" location: "JS frame :: resource://gre/modules/RemoteWebProgress.jsm :: _callProgressListeners :: line 75" data: no]
[task 2019-09-13T18:04:38.933Z] 18:04:38 INFO - " {file: "resource://gre/modules/RemoteWebProgress.jsm" line: 77}]
[task 2019-09-13T18:04:38.933Z] 18:04:38 INFO - _callProgressListeners@resource://gre/modules/RemoteWebProgress.jsm:77:14
[task 2019-09-13T18:04:38.933Z] 18:04:38 INFO - onProgressChange@resource://gre/modules/RemoteWebProgress.jsm:104:10
[task 2019-09-13T18:04:38.934Z] 18:04:38 INFO -
[task 2019-09-13T18:04:38.937Z] 18:04:38 INFO - Console message: [JavaScript Warning: "Loading failed for the <script> with source “http://example.com/browser/dom/serviceworkers/test/utils.js”." {file: "http://example.com/browser/dom/serviceworkers/test/empty_with_utils.html" line: 9}]
[task 2019-09-13T18:04:38.937Z] 18:04:38 INFO - Buffered messages finished
[task 2019-09-13T18:04:38.937Z] 18:04:38 INFO - TEST-UNEXPECTED-FAIL | dom/serviceworkers/test/browser_navigation_process_swap.js | Test timed out -
[task 2019-09-13T18:04:38.937Z] 18:04:38 INFO - GECKO(4209) | JavaScript error: resource://testing-common/PromiseTestUtils.jsm, line 112: uncaught exception: Object
[task 2019-09-13T18:04:38.938Z] 18:04:38 INFO - Console message: [JavaScript Error: "uncaught exception: Object" {file: "resource://testing-common/PromiseTestUtils.jsm" line: 112}]
[task 2019-09-13T18:04:38.938Z] 18:04:38 INFO - GECKO(4209) | MEMORY STAT | vsize 20975013MB | residentFast 1064MB
[task 2019-09-13T18:04:38.938Z] 18:04:38 INFO - TEST-OK | dom/serviceworkers/test/browser_navigation_process_swap.js | took 90028ms
[task 2019-09-13T18:04:38.938Z] 18:04:38 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-09-13T18:04:38.939Z] 18:04:38 INFO - TEST-UNEXPECTED-FAIL | dom/serviceworkers/test/browser_navigation_process_swap.js | Found a tab after previous test timed out: about:blank -
[task 2019-09-13T18:04:38.939Z] 18:04:38 INFO - checking window state
[task 2019-09-13T18:04:38.939Z] 18:04:38 INFO - TEST-START | dom/serviceworkers/test/browser_storage_permission.js
[task 2019-09-13T18:04:42.005Z] 18:04:42 INFO - GECKO(4209) | JavaScript error: resource://testing-common/PromiseTestUtils.jsm, line 112: uncaught exception: Object
Comment 17•5 years ago
|
||
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ce51efd054b8 Remove UAOverride use from mobile/.../browser.js r=snorp https://hg.mozilla.org/integration/autoland/rev/be6959a563f6 Remove UAOverridesBootstrapper r=michal https://hg.mozilla.org/integration/autoland/rev/11f015a3e739 Remove UserAgentOverrides.jsm and UserAgentUpdates.jsm r=michal https://hg.mozilla.org/integration/autoland/rev/7d950fc452fb Remove ua-update.json.in from android build r=snorp
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 18•5 years ago
|
||
Backout by csabou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fe583182f808 Backed out 4 changesets for causing several browser chrome failures. CLOSED TREE
Comment 19•5 years ago
|
||
Backed out 4 changesets (Bug 1513574) for causing several browser chrome failures. Please take a look over those browser chrome failures that turned into permafailures after this bug landed. That happened also on your previous land here: https://tinyurl.com/y28el3c9
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266826086&repo=autoland&lineNumber=17656
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266824748&repo=autoland&lineNumber=12603
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266826095&repo=autoland&lineNumber=13892
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266824590&repo=autoland&lineNumber=3244
Backout link: https://hg.mozilla.org/integration/autoland/rev/fe583182f808dc2c7587c1468b3a54932ed0908a
Comment 20•5 years ago
|
||
Pushed by valentin.gosu@gmail.com: https://hg.mozilla.org/integration/autoland/rev/3ab38d48e289 Remove UAOverride use from mobile/.../browser.js r=snorp https://hg.mozilla.org/integration/autoland/rev/2e0fe7f115ee Remove UAOverridesBootstrapper r=michal https://hg.mozilla.org/integration/autoland/rev/59d191ed3f5b Remove UserAgentOverrides.jsm and UserAgentUpdates.jsm r=michal https://hg.mozilla.org/integration/autoland/rev/4536fef9e911 Remove ua-update.json.in from android build r=snorp
Comment 21•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3ab38d48e289
https://hg.mozilla.org/mozilla-central/rev/2e0fe7f115ee
https://hg.mozilla.org/mozilla-central/rev/59d191ed3f5b
https://hg.mozilla.org/mozilla-central/rev/4536fef9e911
Assignee | ||
Updated•5 years ago
|
Comment 22•5 years ago
|
||
Does this change mean the domain specific override using "general.useragent.override.[DOMAIN]" does not work any more? Because this is what apparently happened. Since the build of September 17th that setting is ignored.
Any workaround?
Assignee | ||
Comment 23•5 years ago
|
||
(In reply to alessino from comment #22)
Does this change mean the domain specific override using "general.useragent.override.[DOMAIN]" does not work any more? Because this is what apparently happened. Since the build of September 17th that setting is ignored.
Any workaround?
That is correct.
There are lots of webextensions that can change the username in a much more reliable way.
Take your pick.
Assignee | ||
Updated•5 years ago
|
Comment 24•5 years ago
|
||
(In reply to Valentin Gosu [:valentin] (he/him) from comment #23)
That is correct.
There are lots of webextensions that can change the username in a much more reliable way.
Take your pick.
Thanks for the response.
It would seem as if "general.useragent.override" is still working, right?
Was there a particular reason why the domain specific part was removed? It did work reliably and was a lot more elegant than to have to install yet another extension.
Assignee | ||
Comment 25•5 years ago
|
||
(In reply to alessino from comment #24)
It would seem as if "general.useragent.override" is still working, right?
Yes, the general browser-wide UA pref still works.
Was there a particular reason why the domain specific part was removed? It did work reliably and was a lot more elegant than to have to install yet another extension.
Yes. This code was quite inefficient, running for all loads, checking if you might have a site-specific UA pref set.
Plus, it had a years old bug where the UA change only happened for the top level load, not for subresource loads.
So it was bad, inefficient and buggy 🙂 We now have better ways for users to perform this change (via webextensions), and for Mozilla to run UA interventions (via webcompat-addon)
Comment 26•5 years ago
|
||
Thats a pity.
It was a very elegant and convenient way to keep the old user interface on Twitter -> https://www.reddit.com/r/Twitter/comments/ce1bea/reverting_back_to_the_old_twitter_interface_for/
Comment 27•5 years ago
|
||
I also think that it was better to just create about:config
entry than installing another extension just to overwrite user-agent, especially if you want to change it for the long term. Because of that, I think that it would be better to implement it back.
Yes. This code was quite inefficient, running for all loads, checking if you might have a site-specific UA pref set.
So, can it be rewritten without those bugs? If addons can already change user-agent, wouldn't be possible to just add another option to "fetch" it from about:config
?
Assignee | ||
Comment 28•5 years ago
|
||
(In reply to Filip Š from comment #27)
I also think that it was better to just create
about:config
entry than installing another extension just to overwrite user-agent, especially if you want to change it for the long term. Because of that, I think that it would be better to implement it back.
While this might work for you, about:config is a tool non-technical users should be using, as changing some things can completely break your browser.
Yes. This code was quite inefficient, running for all loads, checking if you might have a site-specific UA pref set.
So, can it be rewritten without those bugs? If addons can already change user-agent, wouldn't be possible to just add another option to "fetch" it from
about:config
?
Things that are controlled by about:config prefs will sooner or later break. We can only test a small number of configurations, and just as before, having multiple ways of changing the userAgent means not all of them are well tested. And the technical burden of maintaining the code that does this (even assuming it's "bug free") is not worth it.
I agree that it was more convenient just to add a pref for what you wanted, but webExtensions allows you to do that just as easily, and is well tested.
Description
•