350453 - Official branding builds are not using correct homepage/update URLs

Status: RESOLVED FIXED

(Firefox :: General, defect, P1)

Description

[Phil Schwan]

It looks like bug 348165 introduced a change that was not fully absorbed in the l10n document or the code.

Midway through the comment stream, we changed the Getting Started bookmark from http://en-US.www.mozilla.com/en-US/firefox/2.0/ to http://en-US.www.mozilla.com/en-US/firefox/central/

so it's the latter, naturally, for which there is a redirect on the web site.

What was not mentioned anywhere is what effect, if any, this should have on the First Run page, which is presently defined to be http://en-US.www.mozilla.com/en-US/firefox/ff2b2/central/ (404)

We can either change the URL in the document and the code to remove the version and make it consistent with the bookmark, or introduce another redirect to cover that case.

Personally, I think we should do both: add a redirect immediately, so we don't have to respin beta 2, but remove the version number for RC1, unless the intention really is to have the Get Started and First Run pages be different.

beltzner, can you give us your 30-second opinion?  Need to resolve this quickly, so Tim knows how to proceed with b2 QA.

Comment 1

[Phil Schwan]

well silly me: bug 345805 already has the answer, which is that the URLs are correct as they are.

In that case, this bug is very simple -- we just need an extra redirect for beta 2, so that we don't ship with 404s in every locale.

something like /*/firefox/*/central -> /firefox/central should do the trick

Comment 2

[Michael Morgan [:morgamic]]

We're looking at:
RewriteRule (\w{2}(-\w{2}(-mac)?)?)/firefox/(.+)/central/$ http://www.mozilla.com/firefox/central/ [L]

This doesn't conflict with what's said in bug 345805 comment 5?

Comment 3

[Mike Schroepfer]

Note that there appears to be a problem with Firefox.js in the b2 builds.  In particular we are getting the #else part of the #ifdef OFFICIAL_BRANDING section.  This affects:

pref("startup.homepage_override_url","http://www.mozilla.org/projects/%APP%/%VERSION%/whatsnew/");
 67 pref("startup.homepage_welcome_url","http://www.mozilla.org/projects/%APP%/%VERSION%/firstrun/");
 68 // URL user can browse to manually if for some reason all update installation
 69 // attempts fail.
 70 pref("app.update.url.manual", "http://www.mozilla.org/products/%APP%/");
 71 // A default value for the "More information about this update" link
 72 // supplied in the "An update is available" page of the update wizard. 
 73 pref("app.update.url.details", "http://www.mozilla.org/projects/%APP%/");
 74 
 75 // Release notes URL
 76 pref("app.releaseNotesURL", "http://www.mozilla.org/projects/%APP%/%VERSION%/releasenotes/");
 

http://lxr.mozilla.org/mozilla1.8/source/browser/app/profile/firefox.js#51

Comment 4

[Mike Schroepfer]

Let's figure out why we are getting the incorrect values - and determine if we need to respin or setup temp redirects to compensate...

The correct values are:

pref("startup.homepage_override_url","http://%LOCALE%.www.mozilla.com/%LOCALE%/%APP%/%VERSION%/whatsnew/");
pref("startup.homepage_welcome_url","http://%LOCALE%.www.mozilla.com/%LOCALE%/%APP%/%VERSION%/firstrun/");
// URL user can browse to manually if for some reason all update installation
// attempts fail.
pref("app.update.url.manual", "http://%LOCALE%.www.mozilla.com/%LOCALE%/products/%APP%/");
// A default value for the "More information about this update" link
// supplied in the "An update is available" page of the update wizard. 
pref("app.update.url.details", "http://%LOCALE%.www.mozilla.com/%LOCALE%/%APP%/releases/");

// Release notes URL
pref("app.releaseNotesURL", "http://%LOCALE%.www.mozilla.com/%LOCALE%/%APP%/%VERSION%/releasenotes/");

Comment 5

[(not reading, please use [e-mail] instead)]

on my mac, when I build the branch with "ac_add_options --enable-official-branding", I am not getting the official branded prefs, I'm getting the "other" prefs (as schrep pointed out earlier in this bug.)

http://lxr.mozilla.org/mozilla1.8/source/browser/app/profile/firefox.js#51

we are running firefox.js through the preprocessor, but OFFICIAL_BRANDING is not set as a define.

for i in /Users/sspitzer/Desktop/bonecho2/mozilla/browser/app/profile/firefox.js /Users/sspitzer/Desktop/bonecho2/mozilla/browser/app/profile/channel-prefs.js ; \
do /usr/bin/perl /Users/sspitzer/Desktop/bonecho2/mozilla/config/preprocessor.pl  -DAB_CD=en-US -DMOZILLA_INTERNAL_API -DOSTYPE=\"Darwin8.7.1\" -DOSARCH=\"Darwin\" -DBUILD_ID=0000000000 -D_BUILD_STATIC_BIN=1 -DFIREFOX_ICO=\"../../dist/branding/firefox.ico\" -DDOCUMENT_ICO=\"../../dist/branding/document.ico\" -DAPP_VERSION="2.0b2" -DAPP_UA_NAME="Firefox" -DMOZILLA_VERSION=\"1.8.1b2\" -DMOZILLA_VERSION_U=1.8.1b2 -DD_INO=d_ino -DSTDC_HEADERS=1 -DHAVE_ST_BLKSIZE=1 -DHAVE_SIGINFO_T=1 -DHAVE_INT16_T=1 -DHAVE_INT32_T=1 -DHAVE_INT64_T=1 -DHAVE_UINT=1 -DHAVE_UINT16_T=1 -DHAVE_DIRENT_H=1 -DHAVE_GETOPT_H=1 -DHAVE_MEMORY_H=1 -DHAVE_UNISTD_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_X11_XKBLIB_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_MMINTRIN_H=1 -DHAVE_SYS_CDEFS_H=1 -DHAVE_LIBM=1 -DFUNCPROTO=15 -DHAVE_XSHM=1 -DHAVE_RANDOM=1 -DHAVE_STRERROR=1 -DHAVE_LCHOWN=1 -DHAVE_FCHMOD=1 -DHAVE_SNPRINTF=1 -DHAVE_MEMMOVE=1 -DHAVE_RINT=1 -DHAVE_FLOCKFILE=1 -DHAVE_LOCALTIME_R=1 -DHAVE_STRTOK_R=1 -DHAVE_LANGINFO_CODESET=1 -DVA_COPY=va_copy -DHAVE_VA_COPY=1 -DHAVE_I18N_LC_MESSAGES=1 -DMOZ_DEFAULT_TOOLKIT=\"mac\" -DXP_MACOSX=1 -DTARGET_CARBON=1 -DTARGET_API_MAC_CARBON=1 -DMOZ_PHOENIX=1 -DMOZ_BUILD_APP=browser -DMOZ_XUL_APP=1 -DMOZ_ENABLE_CANVAS=1 -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1 -DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_XPINSTALL=1 -DMOZ_JSLOADER=1 -DNS_PRINTING=1 -DNS_PRINT_PREVIEW=1 -DMOZ_XTF=1 -DMOZ_MATHML=1 -DMOZ_UPDATE_CHANNEL=default -DMOZ_FEEDS=1 -DMOZ_STORAGE=1 -DMOZ_SAFE_BROWSING=1 -DMOZ_URL_CLASSIFIER=1 -DMOZ_LOGGING=1 -DHAVE___CXA_DEMANGLE=1 -DMOZ_USER_DIR=\"Mozilla\" -DHAVE_STDINT_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_UINT64_T=1 -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1 -DMOZ_MORK=1 -DMOZ_DLL_SUFFIX=\".dylib\" -DXP_UNIX=1 -DUNIX_ASYNC_DNS=1 -DJS_THREADSAFE=1 -DMOZILLA_1_8_BRANCH=1 -DMOZILLA_LOCALE_VERSION=\"1.8b5\" -DMOZILLA_REGION_VERSION=\"1.8b5\" -DMOZILLA_SKIN_VERSION=\"1.8\"  $i > ../../dist/bin/defaults/pref/`basename $i`; \
done

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: add OFFICIAL_BRANDING to DEFINES

This will ensure that we get the right URL in official builds.

Comment 7

[Reed Loden [:reed]]

CC'ing :bs, as this concerning build config.

If all else fails, I can set up redirects on both www.mozilla.org and www.mozilla.com for whatever is needed.

Comment 8

[(not reading, please use [e-mail] instead)]

on the client side, I think we want to do this:

add:

ifdef MOZ_BRANDING_DIRECTORY
DEFINES += -DOFFICIAL_BRANDING=1
endif

to http://lxr.mozilla.org/mozilla1.8/source/browser/app/Makefile.in

(Similar to what is in http://lxr.mozilla.org/mozilla1.8/source/browser/Makefile.in#54)

I've tested this locally on my mac, where I'm doing official branding, and it produces the correct firefox.js

Comment 9

[(not reading, please use [e-mail] instead)]

Comment on attachment 235760 [details] [diff] [review]
add OFFICIAL_BRANDING to DEFINES

r=sspitzer

I think this issue was introduced with bug #348076 (by dietrich), so you might want his review as well.

Comment 10

[Benjamin Smedberg]

Ugh, we should not have official-branding ifdefs in firefox.js. If we have values that differ per-branding, they should live in a separate file (e.g. firefox-branding.js) that lives in the other-licenses/branding directory).

Comment 11

[Mike Schroepfer]

Preference would be to fix this for b2 and respin as painful as that sounds - since we don't get official branding testing on nightlies the next time we'd test final urls would be in RC1.   If we can get a patch in the 1.8 + the b2 branch by later today we can get full repsins by tomorrow a.m.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Attachment: move prefs to branding specific files

This moves the prefs to a firefox-branding.js file, removing the need to for the OFFICIAL_BRANDING ifdef in firefox.js.

Comment 13

[Benjamin Smedberg]

Comment on attachment 235769 [details] [diff] [review]
move prefs to branding specific files

You also need to add this file to browser/installer/{unix,windows}/packages-static

Comment 14

[:Gavin Sharp [email: [e-mail]]]

I'm going to morph this bug to cover the newly discovered issue. If bug 345805 isn't fixed in time and redirects are still needed, I can file another bug to cover that.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Fixed on the trunk.

mozilla/other-licenses/branding/firefox/Makefile.in 	1.21
mozilla/other-licenses/branding/firefox/pref/firefox-branding.js 	1.1
mozilla/browser/installer/windows/packages-static 	1.88
mozilla/browser/app/firefox-branding.js 	1.1
mozilla/browser/app/Makefile.in 	1.110
mozilla/browser/installer/unix/packages-static 	1.83
mozilla/browser/app/profile/firefox.js 	1.156

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Attachment: branch patch

Comment 17

[Mike Schroepfer]

Gavin - you rock for getting this done so quickly.

In further talking with everyone we are going to do the temprorary redirects for b2 so we don't have to respin (and thus can get beta2 out there as fast as possible).  But we definitely want this in the branch..

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 235785 [details] [diff] [review]
branch patch

This was already reviewed, the flag got lost when changing products.

Comment 19

[Mike Schroepfer]

Comment on attachment 235785 [details] [diff] [review]
branch patch

a=schrep for drivers.

Comment 20

[:Gavin Sharp [email: [e-mail]]]

Checked in on the branch.
mozilla/browser/app/profile/firefox.js 	1.71.2.69
mozilla/other-licenses/branding/firefox/pref/firefox-branding.js 	1.1.2.1
mozilla/other-licenses/branding/firefox/Makefile.in 	1.16.4.5
mozilla/browser/installer/unix/packages-static 	1.50.2.32
mozilla/browser/app/firefox-branding.js 	1.1.2.1
mozilla/browser/app/Makefile.in 	1.85.2.10
mozilla/browser/installer/windows/packages-static 	1.53.2.33