Last Comment Bug 797363 - Remove aol.com user agent override since mail.aol.com is fixed
: Remove aol.com user agent override since mail.aol.com is fixed
Status: RESOLVED FIXED
[qa-]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: 17 Branch
: All All
: -- normal (vote)
: Firefox 19
Assigned To: Dão Gottwald [:dao]
:
:
Mentors:
Depends on: 778408 782453
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-03 08:09 PDT by Dão Gottwald [:dao]
Modified: 2012-10-16 16:34 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
patch (2.62 KB, patch)
2012-10-04 07:30 PDT, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
patch (1.10 KB, patch)
2012-10-10 05:58 PDT, Dão Gottwald [:dao]
felipc: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Dão Gottwald [:dao] 2012-10-03 08:09:40 PDT

    
Comment 1 Dão Gottwald [:dao] 2012-10-04 07:30:03 PDT
Created attachment 667962 [details] [diff] [review]
patch
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-10-05 03:57:52 PDT
Comment on attachment 667962 [details] [diff] [review]
patch

Can you make the initialization of UserAgentOverrides conditional on the presence of general.useragent.override.* prefs, rather than commenting them out like that?

How did you determine that this was fixed?
Comment 3 Virtual_ManPL [:Virtual] - (ni? me) 2012-10-05 06:15:40 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2)
> How did you determine that this was fixed?

Look here
https://bugzilla.mozilla.org/show_bug.cgi?id=778408#c16
Comment 4 Dão Gottwald [:dao] 2012-10-05 06:31:27 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #2)
> Comment on attachment 667962 [details] [diff] [review]
> patch
> 
> Can you make the initialization of UserAgentOverrides conditional on the
> presence of general.useragent.override.* prefs, rather than commenting them
> out like that?

Where? In nsBrowserGlue? In the jsm? Initializing the module without any prefs set is a reasonable thing to do, since the module will monitor the pref branch for live modifications. I just thought we'd rather avoid the tiny overhead, but other consumers might decide differently.

> How did you determine that this was fixed?

I logged in with a dummy AOL mail account.
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-10-07 09:20:01 PDT
(In reply to Dão Gottwald [:dao] from comment #4)
> Where? In nsBrowserGlue? In the jsm? Initializing the module without any
> prefs set is a reasonable thing to do, since the module will monitor the
> pref branch for live modifications. I just thought we'd rather avoid the
> tiny overhead, but other consumers might decide differently.

I meant in nsBrowserGlue. I don't think we need to support live modifications to these prefs at all, really.
Comment 6 Dão Gottwald [:dao] 2012-10-10 05:58:54 PDT
Created attachment 669936 [details] [diff] [review]
patch

This override isn't the last one anymore, so I'm simply removing the pref and leaving the initialization alone.
Comment 8 Ed Morley [:emorley] 2012-10-11 06:57:46 PDT
https://hg.mozilla.org/mozilla-central/rev/4a55eb145a21
Comment 9 Dão Gottwald [:dao] 2012-10-11 08:04:16 PDT
Comment on attachment 669936 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 778408 (tech evangelism made this override unnecessary)
User impact if declined: we expect AOL mail to work either way
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

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