Closed Bug 364297 Opened 18 years ago Closed 17 years ago

Change default home page search and default search engine for Fx 2.0 series

Categories

(Firefox Build System :: General, defect)

2.0 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla3 alpha5

People

(Reporter: baz, Assigned: Gavin)

References

Details

(Keywords: qawanted)

Attachments

(3 files, 11 obsolete files)

22.14 KB, patch
Details | Diff | Splinter Review
21.15 KB, patch
mconnor
: review+
Details | Diff | Splinter Review
21.49 KB, patch
Details | Diff | Splinter Review
Per contract requirements with Google, we need to make Google our default home page and search provider in CJKT locales.

For the home page. This may involve simply changing the DNS entries rather than the builds themselves.

For the search engine, we need to select Google as the default.
Flags: blocking1.8.1.2+
Flags: blocking1.8.0.10+
Assignee: nobody → gavin.sharp
Flags: blocking1.8.0.10+
Bug needs to remain confidential until marketing gives us the OK and prepares an FAQ for the CJKT market about this change. Also, need to inform the broader Yahoo and Google teams about the change.

Also, note that we should NOT be migrating existing Fx 2.0.0.0/2.0.0.1 users to these new settings. Existing user preferences should hold. Only fresh downloads of Firefox 2.0.0.2 and new profiles should get the revised defaults.
Attached patch patch (obsolete) — Splinter Review
This should take care of the default search engines - I want to make sure I'm not forgetting anything, though.

The homepages just need a CNAME change, we're already using ab-cd.start.mozilla.com URLs for those. Probably best to file a Server Ops bug for that.
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Firefox 2
I have no idea on how to do the change of homepage provider without changing existing users. Gavin's patch will migrate existing profiles for search engines, too, AFAICT.

Does this affect 1.5.0.x, too?
(In reply to comment #3)
> Gavin's patch will migrate existing profiles for search engines, too, AFAICT.

Oh, I hadn't thought of that. It will affect the order, but not the selected engine.

> Does this affect 1.5.0.x, too?

No, see bug 364296.
I guess we need a proposal that can help ensure:
1) Existing Firefox 1.5.0.x users who did not change the default from existing Yahoo CJKT are still directed there.
2) Upgraded Firefox 2.0.0.x to 2.0.0.2 users still see the older Yahoo defaults
3) New Firefox 2.0.0.2 users see the newer Google default settings

Thoughts?
Gavin or others, do we have a proposal for how we achieve this?
(Also removed confidential status)
Group: mozillaorgconfidential
I don't think we can, honestly.

We used the default branding for CJKT for yahoo, and we'd have to maintain that, including an update channel, for the rest of our lives, in addition to another branding for google, which we'd offer for download then.

As changing search order is easy in 2.0, can we just let the default search order slip?

As for startpages, I'd suggest to just change the DNS and make those pages google pages, converting existing users, too. In return, we could offer a single snippet only that advertises how to get to the yahoo page for CJKT.
I will inform the localizers in Japan, Korea, Taiwan and China of the upcoming change as they will be impacted and we may ask for their support in the localization of the release notes (specific to CJKT.)

Basic: are we obligated by our contract with Yahoo to continue showing existing users the Yahoo home page and default search engine?
Err, sorry, I meant to say:

Basil: are we obligated by our contract with Yahoo to continue showing existing users the Yahoo home page and default search engine?

One way to do this is to change the names of the search engine preferences (and the domain of the home page) for 2.0.0.2 and later versions but respect the old values, if present.
I did some digging an here's what I suggest we do.
1) Myk, we need to preserve the home page and search engine settings for existing users. We can't switch them from Yahoo to Google. This means that when we apply the Fx 2002 MAR to Fx 2001 users, they need to retain their settings. I think this is OK since those choices are encoded in their user profile.
2) For existing users, we'll leave their home pages pointing to ab-cd.start.mozilla.com
3) For Fx 2002, we need to set the default home page to ab-cd.start2.mozilla.com {where ab-cd is ko-kr for Korean, cn-zh for Simplified Chinese, cn-tw for Traditional Chinese and ja-jp for Japanese}
4) I have filed a Server Ops request (https://bugzilla.mozilla.org/show_bug.cgi?id=366881) to create these four new DNS entries and point them to Google
5) Gavin/Axel, I need to ensure that the patch you created will set the default search engine for Fx 2002 to Google. Order of the engines in the list is NOT critical. But, if it is possible to change Fx 2002 so that Google appears first, then Yahoo, all the better.

Let me know if this addresses all our outstanding issues.

Thanks.
I totally agree this changes.
But don't forgot "keyword" function, the feature is important for Asia users.  
(In reply to comment #11)
> where ab-cd is ko-kr for Korean, cn-zh for Simplified
> Chinese, cn-tw for Traditional Chinese and ja-jp for Japanese}

Simplified Chinese -> zh-cn
Traditional Chinese -> zh-tw
I'd like to reamphisize that the default startpage as well as the default search engine are only stored in the profile if they got changed.

In my install here, I didn't, and the pref for the startpage points to resource:/browserconfig.properties.
http://lxr.mozilla.org/mozilla/source/browser/app/profile/firefox.js#211 makes the default search engine pref point to region.properties.

If we were to keep that setting, we had to move the 2.0.0.2 builds we ship over to a different update channel, and maintain our builds, the ex-yahoo-default builds, in addition to any other partner builds we may still do for the region.
We also have to change these prefs:

- keyword.URL (location bar search, or I'm Feeling Lucky)
- browser.contentHandlers.types.* (the order of feed readers)
Attached patch revised patch (obsolete) — Splinter Review
Attached patch revised patch reloaded (obsolete) — Splinter Review
Ooops, don't forget the special ja-JP-mac locale!
Attachment #251424 - Attachment is obsolete: true
Attachment #251427 - Attachment is patch: true
Attachment #251427 - Attachment mime type: application/octet-stream → text/plain
(In reply to comment #11)

> 1) Myk, we need to preserve the home page and search engine settings for
> existing users.

I understand that, I was just wondering whether it's because of a contractual obligation or because we think it's the best user experience (or both).


(In reply to comment #14)

> I'd like to reamphisize that the default startpage as well as the default
> search engine are only stored in the profile if they got changed.

Hmm, indeed, this makes my suggested solution in comment 10 unworkable.
1) DNS pointers have been setup such that *.start2.mozilla.com all point to www.google.com
2) Kohei is correct in that we need to modify the keyword.URL in the builds from Yahoo to Google (keyword.URL=http://www.google.com/search?ie=UTF-8&oe=UTF-8&sourceid=navclient&gfns=1&q=)
3) For the feed readers, is Yahoo first on the list for CJKT builds? If so, then we should probably modify that
4) Myk, as for migrating users. I don't believe it's contractual but it's what Mitchell and Lilly had talked about directly with Yahoo. Apparently, there is some history here where previous Yahoo users were wholesale migrated (Fx 1.5?) to Google with significant loss of goodwill from Yahoo Corp and the users. Generally, we believe that it's the best user experience to not change people's home page without their consent.

That being said, if we claim that users set "Firefox" as their home page (independent of co-branding), then we should be able to change the co-branding without ill effect on the users. That is, these are "Mozilla Firefox users". But at this point, I don't think we can pursue this avenue.

Most importantly: we currently don't have a solution that doesn't migrate existing haven't-change-the-default users automatically. Any last suggestions or do we have to pursue non-technical options?
(In reply to comment #19)
> 3) For the feed readers, is Yahoo first on the list for CJKT builds? If so,
> then we should probably modify that

Bloglines is the first one, so it would be ok.
Attached patch l10n patch (obsolete) — Splinter Review
This includes the changes from "revised patch reloaded" (though I'm not sure all of them are needed - do we really want to change the order of the Yahoo/Google RSS readers if they're not even 1&2?), and adds a special "don't change default homepage" pref that's used in the Firefox patch I'm going to attach next.
Attachment #249256 - Attachment is obsolete: true
Attachment #251427 - Attachment is obsolete: true
Attached patch browser patch (obsolete) — Splinter Review
This has not been well tested, nor can I say that I've really thought through all of the possible things that could go wrong with this approach, but based on discussion with mconnor I think this is about the only way we can get the behavior we want. This certainly isn't pretty. The basic approach is that we change the current default CNAMES (start.mozilla.com) to all point to Google. Then, we make the start2.mozilla.com CNAMES point to Yahoo. This code runs once after a version bump, and if it detects an "existing" profile (for which a previous value of the mstone pref is set), it checks whether the default homepage is being used, then checks to make sure it isn't overridden by e.g. a partner build, and if both those conditions are true, sets a user pref to the value of the override pref, if that pref exists. The previous patch makes the override pref exist for CJKT, and makes it's value point to start2.mozilla.com. This means that existing users will now have their homepages set to start2.mozilla.com, while new users will point to start.mozilla.com. This requires changing the start2 CNAMES created in bug 366881 to point to Yahoo instead of Google. This will mean that if users use the safe mode "reset my prefs" option, their default homepage will revert to the shipped-build default (Google), but I think that's an edge case that we can tolerate.

I'd very much like feedback on this proposed approach, especially with problems this may cause for future updates or migration, which I haven't given much thought to. The intention is to land this patch on the 1.8.1 branch only, and just let trunk users deal with the changes.
Comment on attachment 251737 [details] [diff] [review]
browser patch

I'm highly dubious on this patch. It'll require us to track much closer still what people put into firefox-l10n.js, and as most of them copy and paste, I wouldn't be surprised to see locales breaking this, or trying to break this.

And I'm pretty sure it'd break new users on the update from 2.0.0.2 to 2.0.0.3.

Re #19, migrating users. As this bug sadly shows, the homepage behavior isn't specified outside of the code. If we take the impl to be the specification, it's specified that users that don't change their home page get directed to the currently shipped homepage for their installed locale.

I guess there's a good reason to change that spec, as we see, and I'm eager to include the lessons learned here into the new-features doc, but I'm not sure if we should hack around that in a hurry.

Sadly, we don't have a minor update in the middle in which we could fix the impl to copy the default homepage into the profile's pref, if we consider that to be the expected behaviour.
> 3) For the feed readers, is Yahoo first on the list for CJKT builds? If so,

I cannot confirm as I am in China and the net connection is very slow, but for ja-JP, I believe Yahoo! Japan is the default.  Kohei can you please confirm?
(In reply to comment #24)
> > 3) For the feed readers, is Yahoo first on the list for CJKT builds? If so,
> 
> I cannot confirm as I am in China and the net connection is very slow, but for
> ja-JP, I believe Yahoo! Japan is the default.  Kohei can you please confirm?

Yes, that's right.

zh-CN/zh-TW: Bloglines, My Yahoo!, Google
ko: HanRSS, Daum, Bloglines, My Yahoo!, Google
ja/ja-JP-mac: My Yahoo!, My Yahoo!, Bloglines, Hatena, livedoor, goo
Err, this is the correct order. We have six feed readers and My Yahoo! is at the top of the list.

ja/ja-JP-mac: My Yahoo!, Google, Bloglines, Hatena, livedoor, goo

I have tested the search engine stuff and made a possible solution. It works for me but reviews and tests required. Please take a look...

b.s = browser.search

1. Add "b.s.previousEngine" pref for CJKT locale
2. Set "b.s.selectedEngine" for all users at browser startup if it is blank. This means that the default search engine is used. This value will be copied from
  - "b.s.previousEngine" for CJKT
  - "b.s.defaultenginename" for the rest of us
3. Don't clear "b.s.selectedEngine" pref when users reselect the default engine
(In reply to comment #23)
> (From update of attachment 251737 [details] [diff] [review])
> I'm highly dubious on this patch. It'll require us to track much closer still
> what people put into firefox-l10n.js, and as most of them copy and paste, I
> wouldn't be surprised to see locales breaking this, or trying to break this.

I'm not very fond of it either, but it's probably the only way we can do what this bug asks for. Frankly, I'm much rather not take this (rather large) risk, and instead just change existing users and new users over to something similar to the current Firefox start page.

> And I'm pretty sure it'd break new users on the update from 2.0.0.2 to
> 2.0.0.3.

Hmm, yeah. I guess we would need to limit this behavior to cases where the previous build is 2.0.0.1 or 2.0...
Another way to distinguish between new and existing profiles would be to change the name of a preference that does get copied to the user's profile (f.e. change the extensions.lastAppVersion pref to extensions.lastAppVersion2002).

This has the advantage that we don't have to rely on something new happening on first run.

A secondary advantage is that the name change persists over time, so we can continue to distinguish between profiles that existed prior to 2.0.0.2 and those created afterwards, which would come in handy if we found a serious bug in the changes we made for 2.0.0.2 that we want to fix in 2.0.0.3.

Of course, with the mstone approach, we could probably just set a separate pref to accomplish this.
Gavin:  What's left to do in this bug?  If we want this in time for 2.0.0.2, we need patches reviewed and approval requests set quickly.  Which one of the patches/approaches look best for the upcoming release?

I'm still trying to find a better solution than what's currently posted. I'll hopefully have a better patch by tomorrow or the day after.
Whiteboard: needs patch
Attached patch patch (obsolete) — Splinter Review
The approach I took in this patch is the following:

1) Create a new per-profile pref folder that takes precedence over all current
   defaults except for extension defaults (involves modifying the
   NS_APP_PREFS_DEFAULTS_DIR_LIST definition to include a new profile
   subdirectory which is then used by pref_InitInitialObjects)

   Pref service currently loads prefs in the following order:
   Hardcoded:
     <appdir>/greprefs

   NS_APP_PREF_DEFAULTS_50_DIR:
     <appdir>/defaults/pref/

   NS_APP_PREFS_DEFAULTS_DIR_LIST:
     <appdir>/defaults/preferences/
     <profile>/pref-overrides              <--- added by this patch
     <extension pref dirs>

   My addition is to NS_APP_PREFS_DEFAULTS_DIR_LIST, before extension pref
   dirs, but after <appdir>\defaults\preferences.

3) Use browser-specific code to, at run-time, detect whether we're launching a
   new build with either a new profile or an existing profile, and use it to
   copy a existing-profile-defaults.js default pref file, to the new profile-
   override pref folder added in 1). This code then tells the pref service to
   reload it's default prefs from files so that the new defaults take effect
   immediately.

This means that the current defaults need to be moved to a localized
existing-profile-defaults.js file, and the standard defaults can simply be
changed to the new values.

This should deal with people using "reset all defaults" from the safe mode
dialog (the new-profile defaults would take effect, since mstone is empty) as
well as people reverting to an old build (the older build's existing-profile.js
would be copied at first startup).

Tests run:
1) Install 2.0.0.1, create a profile, "upgrade" to a patched build.
   - Check that old defaults remain intact, and that user preferences from
     2.0.0.1 aren't removed.
   - No Error Console messages, default pref file successfully copied to
     profile, pref defaults are correct
2) Load 2.0.0.1 with the same profile (or change mstone profile pref to a
   different value manually), change the patched build's "existing profile"
   defaults, then load the patched build again (simulates an upgrade with a
   pref override file already in place, e.g. 2003->2004)
   - No Error Console messages, check that new pref overrides are in place.
3) Start patched build in safe mode, select "reset all my preferences".
   - Verify that defaults are the new-profile defaults and not the existing-
     profile defaults.
4) Create new profile with the patched build.
   - Verify that are the new-profile defaults and not the existing-profile
     defaults.

This patch depends on changes to /l10n files, I'll attach that patch next.
Attachment #251735 - Attachment is obsolete: true
Attachment #251737 - Attachment is obsolete: true
Attachment #251799 - Attachment is obsolete: true
Attachment #252951 - Flags: superreview?(benjamin)
Attachment #252951 - Flags: review?(mconnor)
Attached patch l10n patchSplinter Review
This includes changes to the search engine order, default selected search engine, feed readers and homepages. This patch ideally should be closely reviewed by the CJKT localizers to ensure no mistakes were made. I copied the values from the locales' current region.properties into existing-profile-defaults.js, and then made the changes to the "normal" defaults like the earlier patches in this bug.
(In reply to comment #33)
> Created an attachment (id=252953) [details]
> l10n patch

It looks good. (ja and ja-JP-mac locales)
Thanks for verifying the ja/ja-JP-mac parts of the l10n patch, Kohei. Could the zh-TW/zh-CN/ko localizers please also check this patch to make sure everything looks OK?
Comment on attachment 252951 [details] [diff] [review]
patch

>Index: browser/base/Makefile.in

> ifndef MOZ_BRANDING_DIRECTORY
> libs locale::
> 	$(INSTALL) $(srcdir)/content/browserconfig.properties $(DIST)/bin
>+	$(INSTALL) $(srcdir)/content/old-homepage-default.properties $(DIST)/bin
> 
> install::
> 	$(SYSINSTALL) $(srcdir)/content/browserconfig.properties $(DESTDIR)$(mozappdir)
>+	$(SYSINSTALL) $(srcdir)/content/old-homepage-default.properties $(DESTDIR)$(mozappdir)
> endif

Can you make all of these $(SYSINSTALL) $(IFLAGS1) as in bug 364599.

>Index: toolkit/xre/nsXREDirProvider.cpp

>+#define PREF_OVERRIDE_DIRNAME "pref-overrides"

I would prefer simply "preferences".

>+      else if (!strcmp(aProperty, NS_APP_PREFS_OVERRIDE_DIR)) {
>+        rv = mProfileDir->Clone(getter_AddRefs(file));
>+        rv |= file->AppendNative(NS_LITERAL_CSTRING(PREF_OVERRIDE_DIRNAME));
>+        rv |= EnsureDirectoryExists(file);

Why do we need to ensure the directory exists? I would prefer to avoid creating empty directories. Or are we assuming people only ask for this key if they want to write a file to the directory?
Attachment #252951 - Flags: superreview?(benjamin) → superreview+
(In reply to comment #33)
> Created an attachment (id=252953) [details]
> l10n patch

ko patch is fine too.
Attached patch patch v2Splinter Review
(In reply to comment #36)
> Can you make all of these $(SYSINSTALL) $(IFLAGS1) as in bug 364599.

Done.

> >Index: toolkit/xre/nsXREDirProvider.cpp
> 
> >+#define PREF_OVERRIDE_DIRNAME "pref-overrides"
> 
> I would prefer simply "preferences".

Changed.

> >+      else if (!strcmp(aProperty, NS_APP_PREFS_OVERRIDE_DIR)) {
> >+        rv = mProfileDir->Clone(getter_AddRefs(file));
> >+        rv |= file->AppendNative(NS_LITERAL_CSTRING(PREF_OVERRIDE_DIRNAME));
> >+        rv |= EnsureDirectoryExists(file);
> 
> Why do we need to ensure the directory exists? I would prefer to avoid creating
> empty directories. Or are we assuming people only ask for this key if they want
> to write a file to the directory?

Yes, I made that assumption. The two pieces of code that request this key are simplified by not having to check for its existence.

This patch also adds some code to remove old profile pref overrides before copying the new one (in copyPrefOverride()), so that test 2 from comment 32 passes. For some reason that part didn't make it into the previous patch, even though it was included in the build I tested.
Attachment #252951 - Attachment is obsolete: true
Attachment #253381 - Flags: review?(mconnor)
Attachment #252951 - Flags: review?(mconnor)
mconnor:  can you please review gavin's patch asap?  thanks.
Whiteboard: needs patch → needs r=mconnor
Comment on attachment 253381 [details] [diff] [review]
patch v2

>+  // Remove the pref-overrides dir, if it exists
>+  try {
>+    var fileLocator = Components.classes["@mozilla.org/file/directory_service;1"]
>+                                .getService(Components.interfaces.nsIProperties);
>+    const NS_APP_PREFS_OVERRIDE_DIR = "PrefDOverride";
>+    var prefOverridesDir = fileLocator.get(NS_APP_PREFS_OVERRIDE_DIR,
>+                                           Components.interfaces.nsIFile);
>+    prefOverridesDir.remove(true);

this used to crash, at least on some platforms, if the file didn't exist.  That said, we should only throw if we fail to remove something that exists, so if (file.exists()) file.remove()

so this is a good approach, just needs some solid testing.  I'm assuming this is 2.0 branch only, for now?
(In reply to comment #40)
> said, we should only throw if we fail to remove something that exists, so if
> (file.exists()) file.remove()

The directory service provider ensures the directory exists when it's retrieved, so no need for this (as discussed on IRC).

> so this is a good approach, just needs some solid testing.  I'm assuming this
> is 2.0 branch only, for now?

I think it should land on the trunk too, actually, but it can't really bake there since most of this code only takes effect when you run it in a profile from a build with a different version.
Comment on attachment 253381 [details] [diff] [review]
patch v2

oops, forgot to mark this
Attachment #253381 - Flags: review?(mconnor) → review+
Attachment #253381 - Flags: approval1.8.1.2?
Comment on attachment 253381 [details] [diff] [review]
patch v2

Approved for 1.8 branch, a=jay.  Gavin:  Please let QA know what we should test to make sure we get all the bases covered in verifying this.  Thanks!
Attachment #253381 - Flags: approval1.8.1.2? → approval1.8.1.2+
I've landed both patches on the 1.8 branch. I'm still looking to get zh-TW and zh-CN localizer approval on the l10n patch.
Keywords: fixed1.8.1.2
I've pinged the zh-TW and zh-CN localizers and have cc'd Gavin.  
It would be great to get users of these localizations and any other interested testers to test the next nightly builds that include my patch to ensure that things are working as they should. Linux/Mac testing would be particularly appreciated. Here's an updated list of some of the basic tests I've run, as inspiration for anyone that wants to help out:

"Standard upgrade" test:
1) Install 2.0.0.1, use it to create a new profile.
2) Note the values of the following prefs:
   - home page (browser.startup.homepage)
   - default feed reader order (browser.contentHandlers.types.*)
   - search engine dropdown order and selected engine
     (browser.search.defaultenginename, browser.search.order.*)
   - URL bar keyword search URL (keyword.URL)
   
  I've written a simple extension that lists all of these preferences and their
  values, you can get it at: http://people.mozilla.org/~gavin/364297.xpi.
  To use the extension once it is installed, enter
  "chrome://364297/content/364297.xul" into the location bar and press enter.
  You can then copy/paste the text in the first box to a text
  editor so that it can be compared to the new build in step 4).
3) Change a pref that isn't affected by my patch.
   - I used the "check for default browser at startup" pref since it's easily
     noticed at startup and changing it only requires you to uncheck the
     checkbox in the startup dialog.
4) "Upgrade" to a patched build and start browser.
   - Ensure there are no Error Console messages (with
     javascript.options.showInConsole set to true)
   - Ensure that the profile directory now contains a file named
     "existing-profile-defaults.js" in a preferences/ subdirectory.
5) Check the values of the prefs from 3) and ensure they match the recorded
   values. Also ensure that the pref changed in 3) wasn't reset to default.
   - Note: about:config will show the prefs encoded as "data:" URIs, so using
     my extension's "compare" feature works best.

"Future upgrade" test:
(simulates an upgrade with a pref override file already in place
 e.g. 2003->2004)
1) Load 2.0.0.1 with a profile that's been launched in the patched build.
   - Or -
1) Using about:config, change the browser.startup.homepage_override.mstone pref
   so that it no longer represents the installed version.
2) Change the patched build's "existing profile" defaults, by editing the
   build's <appdir>/defaults/existing-profile-defaults.js and
   <appdir>/old-homepage-default.properties. Use recognizable values so that
   you can easily idenfity them later on.
3) Start the patched build.
   - Ensure there are no Error Console messages (with
     javascript.options.showInConsole set to true)
   - Check that the values changed in 2) are accurately represented in the
     new pref values (you can use my extension for this).

"Safe mode" test:
1) Start patched build in safe mode, select "reset all my preferences".
   - Verify that defaults are the new-profile defaults (Google) and not the
     existing-profile defaults (Yahoo).

"New profile" test:
1) Create new profile with the patched build.
   - Verify that defaults are the new-profile defaults (Google) and not the
     existing-profile defaults (Yahoo).
Keywords: qawanted
Whiteboard: needs r=mconnor
Thank you ,Gen.
I've viewed the patch. And I found Google Reader's url is wrong.
wrong:
browser.contentHandlers.types.1.uri=http://fusion.google.com/add?feedurl=%s
correct:
browser.contentHandlers.types.1.uri=http://www.google.com/reader/view/feed/%s

Can fix it conveniently?


Jose, can you file a new bug for that issue? This patch didn't actually change that value, and fusion.google.com is what was used for 2.0.0.1 as far as I can tell.
Jose- I believe the fusion.google.com url is correct because google wishes to give the user a choice between putting the RSS feed into Google Reader or Google Homepage. 
(In reply to comment #33)
> Created an attachment (id=252953) [details]
> l10n patch

looks good for zh-CN.
Maybe it should be better if we can test a nightly build.

(In reply to comment #50)
> looks good for zh-CN.
> Maybe it should be better if we can test a nightly build.

You can test the builds in http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8-l10n/, they all have this patch now.
Verified fix on CJKT (1.8 l10N branch) for "standard upgrade" , "future upgrade", and "new profile" tests.  Each of the 4 locales demonstrated:
- RSS feeder match whats in comments above
- bookmarks and configurations and extensions are retained upon upgrade
- Search engine converts from yahoo! to Google when profile rolls over
- changing a config value gets retained after the update

Update testing was ran with gavin's extension and following the MAR update posted at: http://wiki.mozilla.org/Software_Update:Manually_Installing_a_MAR_file

a little mix up in my comments during verification.  

I meant to say verified:
- existing profile for search toolbar and launch page from prior build will be retained after update (eg. Yahoo default search engine remains Yahoo default search engine)
- launching a new profile post update will now be using the Google search engine and launch page.
Assignee: gavin.sharp → nobody
Status: ASSIGNED → NEW
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
(In reply to comment #54)
> Hey forks! I found yet another Yahoo! branding to be rewritten before 2.0.0.2.
> In the migration wizard.

Thankfully, none of the locales shipped in 1.8 set homePageOptionCount to anything other than 1, so as far as I can tell that code will never be run, and we don't need to worry about that string. mconnor, can you confirm?
(In reply to comment #54)
> Entries below are OK? 
> http://lxr.mozilla.org/l10n-mozilla1.8/source/ko/other-licenses/branding/firefox/brand.properties

야후! means Yahoo! to be rewritten.

And as always, don't forget ja-JP-mac.
http://lxr.mozilla.org/l10n-mozilla1.8/source/ja-JP-mac/other-licenses/branding/firefox/brand.properties

(In reply to comment #55)
> Thankfully, none of the locales shipped in 1.8 set homePageOptionCount to
> anything other than 1, so as far as I can tell that code will never be run, and
> we don't need to worry about that string. mconnor, can you confirm?

No, there IS a wizard page to choose my home page. Tested with 2.0.0.1.
Attached image Migration Wizard - Homepage (obsolete) —
Screenshot.
(In reply to comment #56)
> No, there IS a wizard page to choose my home page. Tested with 2.0.0.1.

Oh, I'm sorry, I misread the code. It's in an else clause that is indeed used. That means that with current RC builds, new CJKT users who use the profile migrator and select "Firefox Start" as their homepage will still receive a Yahoo start page.
Attached patch CJKT patch for the migrator (obsolete) — Splinter Review
This patch should fix three things:
1) ko and zh-TW have broken migrators, because they're missing the homePageSingleStartMainURL key
2) Revert the strings to the old "Powered by Google" equivalents as needed.
3) Change the values of the homePageSingleStartMainURL to the "start2" URL

I took the strings from previous versions of the files, for ja, ja-JP-mac, and zh-CN, and ko. zh-TW's string seems to already be the Google version (but it's migration dialog is broken anyways, due to 1). 

Kohei, can you review the ja/ja-JP-mac changes? This really should get review from the zh-CN, zh-TW, and ko localizers too.
Attachment #255652 - Flags: review?(kohei.yoshino.bugs)
(In reply to comment #59)
> Kohei, can you review the ja/ja-JP-mac changes?

Looks good.
Attachment #255652 - Flags: review?(kohei.yoshino.bugs)
Channy, Jose Sun- can you please check this patch?

Also, who is the zh-CN localizer on this bug? 
zh-TW: looks good.(In reply to comment #59)
> Created an attachment (id=255652) [details]
> CJKT patch for the migrator
> This patch should fix three things:
> 1) ko and zh-TW have broken migrators, because they're missing the
> homePageSingleStartMainURL key
> 2) Revert the strings to the old "Powered by Google" equivalents as needed.
> 3) Change the values of the homePageSingleStartMainURL to the "start2" URL

zh-TW: all strings looks ok.
Wen Shaohua is in charge for zh-CN. Can you have a look at attachement 255652?
Removing keyword for Gavin. This bug has another patch that needs to land on the branch.
(In reply to comment #59)
> Created an attachment (id=255652) [details]
> CJKT patch for the migrator
> 
> This patch should fix three things:
> 1) ko and zh-TW have broken migrators, because they're missing the
> homePageSingleStartMainURL key
> 2) Revert the strings to the old "Powered by Google" equivalents as needed.
> 3) Change the values of the homePageSingleStartMainURL to the "start2" URL
> 
> I took the strings from previous versions of the files, for ja, ja-JP-mac, and
> zh-CN, and ko. zh-TW's string seems to already be the Google version (but it's
> migration dialog is broken anyways, due to 1). 
> 
> Kohei, can you review the ja/ja-JP-mac changes? This really should get review
> from the zh-CN, zh-TW, and ko localizers too.
> 

I confirmed. Sorry for late to check this bug because of vacation in lunar New Year's Day!
removed verified1.8.1.2 keyword as well, to make sure we re-verify this bug following the migration path for CJKT builds.
Keywords: verified1.8.1.2
In  CJKT patch for the migrator, message "Firefox 를" is wrong korean syntax.
It is fixed with "Firefox를"

Follow patch is right.

-homePageSingleStartMain=야후!의 빠른 검색 페이지로 Firefox를 시작합니다.
+homePageSingleStartMain=구글의 빠른 검색 페이지로 Firefox를 시작합니다.
Attached patch updated CJKT migration patch (obsolete) — Splinter Review
JoungKyun.Kim, could you please verify this new patch?
Attachment #255652 - Attachment is obsolete: true
Attachment #255810 - Flags: review?(admin)
Gavin, on a mac, I can read JoungKyun.Kim's patch allright, could we use that utf-8 encoded string instead of the unicode-escaped stuff that we used to have back then? That way, we can at least do visual comparison without being able to read Korean.

Other than that, I'm giving a GO on landing attachement 255810, independent on whether we got all localizers to sign it off. That's unfortunate, but I'm doing the bite-the-bullet here, and now.
Comment on attachment 255810 [details] [diff] [review]
updated CJKT migration patch

Approved for 1.8 l10n branch, a=jay/r=axel.

Gavin:  Please land this patch ASAP.  No need to talk to Axel as I mentioned on IRC, since he left a comment here.  But he's ok with the patch as-is, so let's get this in.  Thanks!
Attachment #255810 - Flags: approval1.8.1.2+
(In reply to comment #69)
> Gavin, on a mac, I can read JoungKyun.Kim's patch allright, could we use that
> utf-8 encoded string instead of the unicode-escaped stuff that we used to have
> back then? That way, we can at least do visual comparison without being able to
> read Korean.

The problem with that on my side is that I don't actually have the fonts to display those characters, so I can't actually visually compare them unless they're escaped. I'm going to land this patch as is. I've verified that it matches JoungKyun.Kim's patch using the JS console.
l10n/ko/other-licenses/branding/firefox/brand.properties 	1.1.2.9
l10n/ja/other-licenses/branding/firefox/brand.properties 	1.1.2.5
l10n/ja-JP-mac/other-licenses/branding/firefox/brand.properties 	1.1.2.6
l10n/zh-TW/other-licenses/branding/firefox/brand.properties 	1.4.2.6
l10n/zh-CN/other-licenses/branding/firefox/brand.properties 	1.1.2.4
Keywords: fixed1.8.1.2
(In reply to comment #68)
> Created an attachment (id=255810) [details]
> updated CJKT migration patch
> 
> JoungKyun.Kim, could you please verify this new patch?
> 

Okay. This patch has no problem.
Comment on attachment 255810 [details] [diff] [review]
updated CJKT migration patch

(In reply to comment #73)
> Okay. This patch has no problem.

Thank you for checking. 

The only locale that I haven't heard back from yet is zh-CN. It would still be good to get verification after-the-fact.
Attachment #255810 - Flags: review?(admin)
Attached patch fixed wrong korean syntax (obsolete) — Splinter Review
Sorry. After I confirm "updated CJKT migration patch" patch, I found another wrong syntax on modified patch. So, I attach new patch for korean tree.
Attachment #255810 - Attachment is obsolete: true
(In reply to comment #75)
> Sorry. After I confirm "updated CJKT migration patch" patch, I found another
> wrong syntax on modified patch. So, I attach new patch for korean tree.

Unfortunately I think it's too late - the candidate 2.0.0.2 builds are already complete. You can probably get this fix into a later release (2.0.0.3), though, so I recommend filing a new bug for that.
I tested 2.0.0.2 rc5 ja/ja-JP-mac and confirmed that the Migration Wizard worked well. Welcome back Google!
(In reply to comment #78)
> Yahoo! still remains in Help.

Thanks. We should fix it with 2.0.0.3. I'll update our online help though.
Verified the migration wizard fix on 2.0.0.2 RC5 for CJKT.  

Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-TW; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
- Migration wizard shows "powered by Google" migrator option
- homepage URL: http://zh-tw.start2.mozilla.com/firefox?client=firefox-a&rls=org.mozilla:zh-TW:official

Mozilla/5.0 (Windows; U; Windows NT 5.1; zh-CN; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
- Migration wizard shows "powered by Google" migrator option
- homepage URL: http://zh-cn.start2.mozilla.com/firefox?client=firefox-a&rls=org.mozilla:zh-CN:official

Mozilla/5.0 (Windows; U; Windows NT 5.1; ja; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
- Migration wizard shows "powered by Google" migrator option
- homepage URL: http://ja.start2.mozilla.com/firefox?client=firefox-a&rls=org.mozilla:ja:official

Mozilla/5.0 (Windows; U; Windows NT 5.1; ko; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
- Migration wizard shows "powered by Google" migrator option
- homepage URL: http://ko.start2.mozilla.com/firefox?client=firefox-a&rls=org.mozilla:ko:official
I'll also add in my test results for CJKT, that importing IE homepage instead of Firefox Google homepage, will in fact restore the IE default homepage saved in the preferences.
I believe this has been resolved. Reopen if there are existing issues.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
I'm leaving this because I need to remember to land the patch on the trunk.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I've landed the patch on the trunk, as-is.

mozilla/browser/base/Makefile.in 1.19
mozilla/browser/base/content/old-homepage-default.properties 1.2
mozilla/browser/base/content/safeMode.js 1.9
mozilla/toolkit/xre/nsXREDirProvider.cpp 1.53
mozilla/browser/components/nsBrowserContentHandler.js 1.40
mozilla/browser/installer/unix/packages-static 1.97
mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp 1.16
mozilla/browser/components/dirprovider/nsBrowserDirectoryServiceDefs.h 1.3
mozilla/browser/installer/windows/packages-static 1.108
mozilla/browser/locales/Makefile.in 1.51
mozilla/xpcom/io/nsAppDirectoryServiceDefs.h 1.22
mozilla/other-licenses/branding/firefox/locales/Makefile.in 1.4
mozilla/other-licenses/branding/firefox/locales/browserconfig.properties 1.4
mozilla/other-licenses/branding/firefox/locales/old-homepage-default.properties 1.2
mozilla/modules/libpref/src/nsPrefService.cpp 1.94
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Attached patch trunk patchSplinter Review
Attachment #255650 - Attachment is obsolete: true
Attachment #255857 - Attachment is obsolete: true
Target Milestone: Firefox 2 → Firefox 3 alpha5
Depends on: 380429
Component: Build Config → General
Product: Firefox → Firefox Build System
Target Milestone: Firefox 3 alpha5 → mozilla3 alpha5
You need to log in before you can comment on or make changes to this bug.