Closed Bug 797363 Opened 9 years ago Closed 9 years ago

Remove aol.com user agent override since mail.aol.com is fixed

Categories

(Firefox :: General, defect)

17 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 19
Tracking Status
firefox17 --- fixed
firefox18 --- fixed

People

(Reporter: dao, Assigned: dao)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #667962 - Flags: review?(felipc)
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?
Attachment #667962 - Flags: review?(felipc)
(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.
(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.
Attached patch patchSplinter Review
This override isn't the last one anymore, so I'm simply removing the pref and leaving the initialization alone.
Attachment #667962 - Attachment is obsolete: true
Attachment #669936 - Flags: review?(felipc)
Attachment #669936 - Flags: review?(felipc) → review+
https://hg.mozilla.org/mozilla-central/rev/4a55eb145a21
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
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
Attachment #669936 - Flags: approval-mozilla-beta?
Attachment #669936 - Flags: approval-mozilla-aurora?
Attachment #669936 - Flags: approval-mozilla-beta?
Attachment #669936 - Flags: approval-mozilla-beta+
Attachment #669936 - Flags: approval-mozilla-aurora?
Attachment #669936 - Flags: approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.