Closed Bug 474528 Opened 16 years ago Closed 13 years ago

IE pops up two download dialog boxes when clicking on download boxes for non-en-US locales

Categories

(www.mozilla.org :: General, defect)

Product:
www.mozilla.org
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: pascalc, Assigned: clouserw)

References

(
URL
)

Details

Attachments

(1 file)

fix?
3.21 KB, patch
Details | Diff | Splinter Review
Steps to reproduce the bug: 1 Go to http://th.www.mozilla.com/th/ with IE (6 or 7) 2 click on the download box Expected result: download dialog pops up Actual result: 2 download dialogs pop up This is due to the javascript code included in the init_download() function launched by the onclick event. http://viewvc.svn.mozilla.org/vc/projects/mozilla.com/tags/production/js/download.js?revision=18534&content-type=text%2Fplain&pathrev=21645 This code was added because of bug #393263 to work around an IE bug preventing the display of the download dialog while redirecting to the transition page. Other locales do not have a transition page therefore it triggers two downloads instead. We could fix that in the productDetails class by not including the onclick event in the generated code but having this code included is actually handy for mozilla-europe.org where I put our specific omniture code for download counts. Another solution would be to diasble the function like that: <script type="text/javascript"> // reset init_download() to avoid two download dialog boxes on IE for locales without a transition page init_download = function(link) {} </script> This could be included in the footer of our localized pages (in-product and landing pages) and directly on pt-BR <body> pages which is a full localization of mozilla.com and thus uses the same heade/footer as en-US. Note that this could partly explain why we have so many people starting downloads and not finishing them and could actually mean that our retention rate is miscalculated, so this is important for both the end-user experience and marketing analysis.
I think your method works. I can add this to pt-BR and th. Do you have a list of other affected locales?
All locales with a landing page are affected, that is 29 locales (like th). It also affects in-product pages like the /xx/firefox/customize but those are unlikely to be displayed by IE users.
Attached patch fix? — — Splinter Review
Isn't this what you made the download_base_url_transition variable for? Can't we just key off that the same way we do for all.html? (seems to work with my minor testing)
Assignee: nobody → clouserw
originally yes, but the download_base_url_transition variable is not called from all of the templates in product-details and I know you wanted to rewrite all of it so I am not sure it's, worth updating it. Plus now we need to add an onclick event on the download box for omniture tracking, it is convenient to use init_download() for that.
One thing I noticed in the duplicate requests we see in the log files mentioned in bug 475810 is that the second request usually has a dmo cookie and referrer string of www.mozilla-europe.com while the first one doesn't. Is there a simple explanation for that?
Daniel, the download.html page is on mozilla.com so the first visit has a referrer from mozilla-europe.org but the second display of this page is forced by the opening of a pop up window when you are already on download.html so there should be no referrer for the second download I think since we are already on mozilla.com
Comment on attachment 358291 [details] [diff] [review] fix? I don't fully understand your comment #4 so I figured I'd r? you with the patch.
Attachment #358291 - Flags: review?(pascalc)
Hi Wil, here are my comments on your patch: - the test you added after the 'switch ($platform)' statement duplicates the one before the same switch statement, they should be merged - you also initialize $_onclick for all cases of the 'switch ($_layout)' test, I think the variable should be set before the switch statement - the creation of $_include_init_download as boolean seems redundant to me with the testing of its value afterwards and setting the onclick value. Since it's either true/false, I think we should just set it in the test before the 'switch ($platform)' statement This is what I would propose for the test before switch($_layout): if (in_array($locale, $this->has_transition_download_page)) { $_download_base_url = $this->download_base_url_transition; $_onclick = "onclick=\"init_download('{$this->download_base_url_direct}?product={$_product}-{$_current_version}&amp;os={$_os_shortname}&amp;lang={$locale}');\""; } else { $_download_base_url = $this->download_base_url_direct; $_onclick = ''; }
more comments, although your patch does resolve this bug, it will break per locale download counting on mozilla-europe.org in omniture (see comment #0) since we need an onclick event on the link to exist to call the omniture code and we are currently redefining init_download for that. It is not unlikely that marketing in the US will also be interested in counting per locale/platform/os clicks on the DL box on mozilla.com in the future too. So actually, what we need is to add a different onclick event when we don't have a transition page, not to have none: if (in_array($locale, $this->has_transition_download_page)) { $_download_base_url = $this->download_base_url_transition; $_onclick = "onclick=\"init_download('{$this->download_base_url_direct}?product={$_product}-{$_current_version}&amp;os={$_os_shortname}&amp;lang={$locale}');\""; } else { $_download_base_url = $this->download_base_url_direct; $_onclick = "onclick=\"count_downloads('{$_product}', '{$_current_version}', '{$_os_shortname}', '{$locale}');\""; } count_downloads() being a js function to add to launch the omniture link counting code.
oremj, would it make sense for mozilla.com or other omniture powered sites to have the count_downloads() function fired up chen clicking the DL box? if so, do you have comments on wil's patch and my remarks ? thanks
Typically I just attach any onclick events in the javascript, but it might be useful to have that function called when clicking downloads. Something like onclick="download_hooks(args...);" also works for me.
I think the best idea would be to remove all this tracking from the library and then each site can put whatever handlers they want on the elements in their own javascript.
Triaging. Pages have long been replaced by new one, we no longer use Omniture which was part of the cause, no other reports and I can't duplicate today, I guess that fixes it.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WORKSFORME
Attachment #358291 - Flags: review?(pascalc)
Component: www.mozilla.org/firefox → www.mozilla.org
Component: www.mozilla.org → General
Product: Websites → www.mozilla.org
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: