Last Comment Bug 591327 - Remove .extra UA pref, possibly add UI for .compatMode.firefox
: Remove .extra UA pref, possibly add UI for .compatMode.firefox
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b1
Assigned To: Robert Kaiser
:
Mentors:
Depends on: 581008 597051
Blocks: 594578
  Show dependency treegraph
 
Reported: 2010-08-27 06:20 PDT by Robert Kaiser
Modified: 2010-09-16 10:28 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1: add pref and turn it on (3.65 KB, patch)
2010-09-03 08:58 PDT, Robert Kaiser
iann_bugzilla: review-
Details | Diff | Splinter Review
v1.1: add help text (5.08 KB, patch)
2010-09-04 06:04 PDT, Robert Kaiser
iann_bugzilla: review+
neil: superreview+
Details | Diff | Splinter Review

Description Robert Kaiser 2010-08-27 06:20:05 PDT
Bug 581008 removed general.useragent.extra.* support and introduced a general.useragent.compatMode.firefox pref for adding a Firefox token to the UA.

We should remove our .extra prefs from default prefs and investigate if we want to expose the compatMode one in the advanced UI.

I think we should leave compatMode.firefox off by default, but giving users easy control of it might be a good idea.
Comment 1 neil@parkwaycc.co.uk 2010-08-31 13:48:10 PDT
You had me worried there; in your blog you seemed to suggest turning it on by default!
Comment 2 Robert Kaiser 2010-08-31 17:47:04 PDT
(In reply to comment #1)
> in your blog you seemed to suggest turning it on by default!

I am now, after thinking about this more, looking at reports from our users, and after losing all Minefield testers in Tech Evang efforts by them switching to "Firefox/x.y" even in nightlies (and the latter was the final part of the puzzle to make me favor having it on by default or even better not making it optional at all).
Comment 3 Robert Kaiser 2010-09-03 08:58:03 PDT
Created attachment 471863 [details] [diff] [review]
v1: add pref and turn it on

As this topic might still be somewhat controversial, I'm requesting both review and super-review on this.

I have made the argument in here and on other places, that "now", i.e. in the Gecko 2.0 timeframe, we would be the only Gecko browser, including preview versions (of e.g. Firefox), that would not send a "Firefox" token, and that means that cases that hurt us already, where people state that "SeaMonkey doesn't work with those websites", will just grow more. Also, Microsoft apparently shipping .NET web frameworks with a broken sniffer that sends invalid XHTML and makes us see a yellow screen of death makes the user experience awful esp. in regions where we need to gain users (e.g. the US).
I hate that we must do it, but I'm convinced that switching this on by default is the only way to not harm our user base from SeaMonkey 2.1 going forward.
Comment 4 neil@parkwaycc.co.uk 2010-09-03 16:05:53 PDT
Comment on attachment 471863 [details] [diff] [review]
v1: add pref and turn it on

>+    <groupbox flex="1">
Nit: flex is wrong.
Comment 5 Robert Kaiser 2010-09-03 16:58:35 PDT
(In reply to comment #4)
> Nit: flex is wrong.

Removed locally, but I think that doesn't warrant attaching a new patch, right? :)
Comment 6 Ian Neal 2010-09-04 05:20:03 PDT
Comment on attachment 471863 [details] [diff] [review]
v1: add pref and turn it on

I can possibly understand why it go on this pref pane as it is what it is sent out in a header, but would it be better under the main advanced pane? Only a suggestion and I'd not fall out over it.
Putting on my other hat, as it is a single additional pref, I would expect the help page to be updated in the same bug and patch.
r- as I'd want to review the updated help page.
Comment 7 Robert Kaiser 2010-09-04 06:04:07 PDT
Created attachment 472148 [details] [diff] [review]
v1.1: add help text

Thanks for the reminder - this one adds help for this entry, I hope the text is understandable and useful, I'm not the best documentation writer as I tend to explain in too many words (I think).
Comment 8 neil@parkwaycc.co.uk 2010-09-04 07:18:42 PDT
Comment on attachment 472148 [details] [diff] [review]
v1.1: add help text

I reread the UA thread on n.d.a.seamonkey and it seems that the balance of opinion is to add the compatMode by default.

I'd prefer s/Announce/Advertise/g (or whatever the U.S. spelling is)

Suggested help text (IanN has final say):

The identifier sent by &brandShortName; to all websites is used for statistics about website usage but also sometimes to expose certain features only to known browsers ("sniffing").

: If this is enabled then &brandShortName; advertises "Firefox" in addition to its own name, so websites work even if they use bad "sniffing" code that enables functionality only for Firefox.
Comment 9 Justin Wood (:Callek) (Away until Aug 29) 2010-09-06 09:28:51 PDT
>>If this is enabled then &brandShortName; advertises "Firefox" in addition to
its own name, so websites work even if they use bad "sniffing" code that
enables functionality only for Firefox.<<

How about:

If this is enabled then &brandShortName; advertises "Firefox" in addition to its own name, this allows websites performing "sniffing" badly to continue to work, rather than break because they use functionality only in Firefox.


But of course, defer to IanN as I am a bad doc writer as well.
Comment 10 Jens Hatlak (:InvisibleSmiley) 2010-09-06 14:01:22 PDT
(In reply to comment #9)
> websites performing "sniffing" badly

Can you do sniffing well? ;-)

> rather than break because they use functionality only in Firefox.

I read that as "functionality only contained in Firefox". If a website relied on functionality only (contained) in Firefox, the pref wouldn't help, so this is misleading. The original version was a bit clearer (enable /for/ Firefox).

I think we should be a bit less technical here and instead explain that the pref allows you to visit/use websites that would otherwise only display or behave correctly in Firefox. Suggestion:

"If this is enabled, &brandShortName; will identify itself as both &brandShortName; and Firefox. This allows websites that check for certain browsers rather than certain functionality to work with &brandShortName;."
Comment 11 Justin Wood (:Callek) (Away until Aug 29) 2010-09-06 14:03:08 PDT
(In reply to comment #10)
> "If this is enabled, &brandShortName; will identify itself as both
> &brandShortName; and Firefox. This allows websites that check for certain
> browsers rather than certain functionality to work with &brandShortName;."

Wow, that is so much better than my own :-)
Comment 12 Ian Neal 2010-09-06 14:28:09 PDT
Comment on attachment 472148 [details] [diff] [review]
v1.1: add help text

(In reply to comment #8)
> Comment on attachment 472148 [details] [diff] [review]
> v1.1: add help text
> 
> I reread the UA thread on n.d.a.seamonkey and it seems that the balance of
> opinion is to add the compatMode by default.
> 
> I'd prefer s/Announce/Advertise/g (or whatever the U.S. spelling is)
I agree, Advertise (or Advertize) sounds better.
> 
> Suggested help text (IanN has final say):
> 
> The identifier sent by &brandShortName; to all websites is used for statistics
> about website usage but also sometimes to expose certain features only to known
> browsers ("sniffing").
I think we need to be more explicit about what "sniffing" is, so perhaps in brackets it should be:
(a practice known as "sniffing")

(In reply to comment #10)
> "If this is enabled, &brandShortName; will identify itself as both
> &brandShortName; and Firefox. This allows websites that check for certain
> browsers rather than certain functionality to work with &brandShortName;."

Yes, a lot better, I think this clarifies things even more:
"If this is enabled, &brandShortName; will identify itself as both
&brandShortName; and also compatible with Firefox. This allows websites that
check for certain browsers rather than certain functionality to work with &brandShortName;."

r=me with those points addressed.
Comment 13 Dave Garrett 2010-09-06 14:47:39 PDT
(In reply to comment #12)
> Yes, a lot better, I think this clarifies things even more:
> "If this is enabled, &brandShortName; will identify itself as both
> &brandShortName; and also compatible with Firefox. This allows websites that
> check for certain browsers rather than certain functionality to work with
> &brandShortName;."

I suggest changing the second usage of the word "certain" to "needed". Thus:

"If this is enabled, &brandShortName; will identify itself as both
&brandShortName; and also compatible with Firefox. This allows websites that
check for certain browsers rather than needed functionality to work with
&brandShortName;."

Avoids repetitive usage of the word and is more explicit as to what it's doing this for, i.e. sites should be checking for what they need not whatever they happen to feel like. ;)
Comment 14 Robert Kaiser 2010-09-07 17:39:41 PDT
Pushed as http://hg.mozilla.org/comm-central/rev/c8b0e716441e with the string fixups (I hope I caught all of them correctly).
Comment 15 Wyatt Childers 2010-09-08 15:44:38 PDT
Guys this causes a major problem with AMO u realize? AMO needs an update if this setting is going to be put on by default in the none testing builds. The problems is that AMO enables the install for Firefox... Is there a quick fix that the AMO people could do? I mean of course the compatibility checker will stop the installation but it makes the entire website seem buggy or I imagine it would give that feel if the user didn't know why this was happening.
Comment 16 Dave Garrett 2010-09-08 15:49:56 PDT
(In reply to comment #15)
I just filed bug 594578 for that.

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