Closed Bug 350453 Opened 14 years ago Closed 14 years ago

Official branding builds are not using correct homepage/update URLs

Categories

(Firefox :: General, defect, P1)

2.0 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 2

People

(Reporter: phil, Assigned: Gavin)

Details

(Keywords: fixed1.8.1)

Attachments

(2 files, 1 obsolete file)

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.
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
Assignee: beltzner → morgamic
Status: NEW → ASSIGNED
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?
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
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/");
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

Attached patch add OFFICIAL_BRANDING to DEFINES (obsolete) — Splinter Review
This will ensure that we get the right URL in official builds.
Attachment #235760 - Flags: review?
Attachment #235760 - Flags: review? → review?(sspitzer)
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.
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 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.
Attachment #235760 - Flags: review?(sspitzer)
Attachment #235760 - Flags: review?(dietrich)
Attachment #235760 - Flags: review+
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).
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.
This moves the prefs to a firefox-branding.js file, removing the need to for the OFFICIAL_BRANDING ifdef in firefox.js.
Attachment #235760 - Attachment is obsolete: true
Attachment #235769 - Flags: review?(benjamin)
Attachment #235760 - Flags: review?(dietrich)
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
Attachment #235769 - Flags: review?(benjamin) → review+
Assignee: morgamic → gavin.sharp
Status: ASSIGNED → NEW
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.
Component: www.mozilla.com → General
Flags: review+
Product: Websites → Firefox
QA Contact: www-mozilla-com → general
Summary: beta2 First Run URL is 404 → Official branding builds are not using correct homepage/update URLs
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → Firefox 2 beta2
Version: unspecified → 2.0 Branch
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
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: blocking-firefox2?
Resolution: --- → FIXED
Attached patch branch patchSplinter Review
Attachment #235785 - Flags: approval1.8.1?
Attachment #235785 - Flags: approval1.8.1?
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..
Target Milestone: Firefox 2 beta2 → Firefox 2
Flags: blocking-firefox2? → blocking-firefox2+
Comment on attachment 235785 [details] [diff] [review]
branch patch

This was already reviewed, the flag got lost when changing products.
Attachment #235785 - Flags: review?(benjamin) → review+
Comment on attachment 235785 [details] [diff] [review]
branch patch

a=schrep for drivers.
Attachment #235785 - Flags: approval1.8.1? → approval1.8.1+
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
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.