Closed Bug 514815 Opened 15 years ago Closed 14 years ago

Add check for out of date flash to welcome page

Categories

(Camino Graveyard :: Product Site, defect)

All
macOS
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Camino2.1

People

(Reporter: chris, Assigned: stuart.morgan+bugzilla)

References

()

Details

Attachments

(1 file, 1 obsolete file)

Bug 512483 adds a check to Firefox's "What's New" page for the Flash Player version. An upgrade notice and link are displayed if there is a newer version of Flash available. Seems like something we should be doing as well. We should be able to adapt the JS in that bug and add it to whatever page users are presented with when they've updated Camino.
Note that we currently support Mac OS X 10.3.9 in 1.6.x, which has a different latest version of Flash from Mac OS X 10.4+ (and web content has no way of getting the OS version in 1.6.x).
Seems like we should:
1) Use OS version if available; before long (yay!) the overwhelming majority of our users should have 2.0.
2) If it's not available, check the current Flash version against the 10.3.9 max (9.0 r246, from what I can tell) and the 10.4+ max. If it's between the two, send them to the normal download page. If it's less than the 10.3.9 max, send them to http://get.adobe.com/flashplayer/otherversions/ (where the dropdown will let them choose by their OS version).
3) If it's exactly the 10.3.9 version, do nothing.

I looked at the current cb.o stats, and 9.0 r246 accounts for only 0.6% of Flash versions there, so I think we'll do extremely well with that approach.
Related, but not really this bug, except for perhaps writing the detection/version registry js in such a way that expanding it could be easy:

We currently have a list of “supported” plug-ins and minimum versions in the setup docs: http://caminobrowser.org/documentation/setup/#find_plugin  

We update this list with each major Camino version (and periodically between releases if warranted, e.g. the new F4M release) to ensure users have the latest versions, or at least versions that are known to fix significant issues (descriptor leaks, CFReadStreamRead crashes, crashes when using Flashblock, other incompatibilities, etc.).

It might be useful in the long run if we offered a page that did a general plug-in check that we could link from the setup page and perhaps, later on, from the new-version page.
Mozilla's working on such a plug-in update page. We shouldn't try to re-invent it and just use their source code when it's ready.
Since Sam hasn't objected to the Flash check on welcome, confirming.

Sam is also the JS guy on the website team, so I suspect this won't happen until he gets around to it, or someone else volunteers a patch ;)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Our current #2 crasher, accounting for 2.15% of 2.0.1 crashes, is a crash in a version of Flash that isn't identified on https://wiki.mozilla.org/CrashKill/Mac_Flash_Identifiers, which means it's either really old (< 9.0.151) or it was some sort of "unreleased" build that people are using. Most users with the crash are 10.4 PPC.

We really need to do something to make it go away :/
Assignee: samuel.sidler+old → samuel.sidler
Or it's Flash Player 10.0.42.34, which has been released but isn't listed on that page.
(In reply to comment #7)
> Or it's Flash Player 10.0.42.34, which has been released but isn't listed on
> that page.

Or, you know, 9.0.47.0, from July of 2007 :P

This is really ridiculous.
http://svn.mozilla.org/projects/mozilla.com/trunk/js/detect-flash.js

We'd need to rewrite the insertion code not to use all the YUI stuff that cbo doesn't use, but at the same time we've have to rewrite the insertion code to fit with cbo's page structure, anyway.  And to do comment 2.
cl also noted today that our #1 crasher (Flash Player@0x92160, bug 520058), accounting for about 3% of all 2.0.1 crashes, is the previous version of Flash (10.0.32.18).
This is really-really-wanted-2.0.2.  As a backup plan, we can add a link on start/welcome to the MoCo page, but the experience would be far better, and the check more effective, if we had it working on cbo.
Flags: camino2.0.2?
Sam, how far from the end of your tabs is this now?
Flags: camino2.0.3?
Flags: camino2.0.2?
Flags: camino2.0.2-
Flags: camino2.0.3? → camino2.0.3-
We need to roll this out ASAP, but it absolutely needs to block the 2.1 website.
Flags: camino2.1+
Flags: camino2.0.4?
Flags: camino2.0.4-
Target Milestone: --- → Camino2.1
(In reply to comment #9)
> http://svn.mozilla.org/projects/mozilla.com/trunk/js/detect-flash.js
> 
> We'd need to rewrite the insertion code not to use all the YUI stuff that cbo
> doesn't use, but at the same time we've have to rewrite the insertion code to
> fit with cbo's page structure, anyway.  And to do comment 2.

Also, that detection code doesn't detect the final part of the version number, which is what we'd want to detect to get people to take crash and security fixes; detecting the version and the revision isn't useful (except for getting that 25% of our users on 9.0 and 10.0 to 10.1; by contrast, only 7% of our users are on the latest 10.1 revision, which hasn't yet had a security update to detect).
I need a brief autofill break so I started looking at this.

(In reply to comment #14)
> Also, that detection code doesn't detect the final part of the version number,
> which is what we'd want to detect to get people to take crash and security
> fixes; detecting the version and the revision isn't useful

I don't think that's true, actually. There are four parts to a Flash version string, and the script detects the first 3. From:
https://wiki.mozilla.org/CrashKill/Mac_Flash_Identifiers
it appears that the 3rd is sufficient to distinguish between everything but different stage beta/preview updates of the same release.

Which is good because there's no way to get the full version of the plugin via JS before Gecko 1.9.2 :P


(Also, I checked the download for 10.3.9, and it's currently 9.0 r280)
Attached file Detection/warning script (obsolete) —
Here's a first cut at the JS to support this. It should handle both 2.0 and 1.6 correctly, and tries to guess the OS for 1.6 as described in comment 2.

The script assumes that there will be a node (I envision an empty div) with id "flashwarning" on the page. If a message is shown it will set that node's class to "visible", so I envision the bare-bones version of the style sheet being:
  #flashwarning {
    display: none;
  }
  #flashwarning.visible {
    display: hidden;
  }
If people don't like that approach, I'm happy to change it to whatever anyone wants. I can also adjust structure as necessary (e.g., if we need the text in a <p> in the node instead of directly inside, for styling purposes).

Notes:
- Only supports OS X, but should work in other browsers (I've tested Camino 1.6 and 2.1a1pre, Chrome 6, and Firefox 3.6).
- I have no idea if we have a JS style for our site, so I just did what I'm used to. I can change if we do have a style.
- Someone will probably want to polish the text.

Someone else should do the HTML/CSS part of this.
Attachment #472441 - Flags: review?
As Smokey pointed out, I misspelled "block" as "hidden" in the CSS :P
Comment on attachment 472441 [details]
Detection/warning script

JS review should go to Sam ;)

http://caminobrowser.org/welcome/index-flash.php is a test page with it deployed and the basic HTML/CSS stubbed in.

Stuart also suggests that we want a header or something like the /start/ messages have.

I think we might want a scary warning bubble, too, like the antiquated versions get.
Attachment #472441 - Flags: review? → review?(samuel.sidler)
We should match the line-height and/or font size, too; I'm not sure off-hand where that's coming in, but the new message text isn't matching.

I've tested 1.6 on 10.3.9, and it works nicely there.
(In reply to comment #19)
> We should match the line-height and/or font size, too; I'm not sure off-hand
> where that's coming in, but the new message text isn't matching.

* the line height for the other block of body text is set on the <p>, line 26 of the stylesheet.

* I think the message should have a bit of highlighting, similar to what is done on /start/ when you run an outdated release build. That styling is set inline via the js:
http://caminobrowser.org/js/ua-messages-start.js

something like this for the stylesheet:

#flashwarning {
  padding: 0 15px;
  background-color: #ff9693;
  border: 2px solid #af0000;
  -moz-border-radius: 12px;
  line-height: 1.4; /* matches line 26 */
  margin-bottom: 2em /* separate it some more form the 'Welcome...' <h2> */
}

PS - script appears to work fine on 10.3.9 and 10.6 after I downgraded the Flash player
I've massacred Stuart's js to give us a h2 and to put the text inside a <p> (which lets us grab all relevant <p> styles automatically), and updated the test page to use philippe's CSS.

I think Stuart's text is fine; we use "urge[d]" elsewhere; I'm not sure I have a strong preference between "strongly recommend" and "urge".

I'm really not sure what to use for the h2 text; Firefox had the heading "You should update Adobe Flash right now" (and then the paragraph text being that Firefox had been updated, but Flash was out-of-date, and that Flash was a free upgrade).

philippe suggested that we might not need a heading at all, since the message is pretty clear, but I'd like a good "summary" of why the user is seeing this block at all.  Or maybe just something like "Be careful!"
If we end up putting this on any pages other than the one people see when launching an updated Camino, we should add -webkit-border-radius too.
(In reply to comment #22)
> we should add -webkit-border-radius too.
Oh, sure, and 'border-radius' as well (webkit nightlies support it now, gecko 2.0 soon probably)
(In reply to comment #22)
> If we end up putting this on any pages other than the one people see when
> launching an updated Camino, we should add -webkit-border-radius too.

(In reply to comment #23)
> Oh, sure, and 'border-radius' as well (webkit nightlies support it now, gecko
> 2.0 soon probably)

I thought about that when I was pasting in the styles; we may as well just make pairing -moz-foo with -webkit-foo (and standardized foo, where applicable) a site-wide policy (we do it other places already, but making it site-wide should make us not forget).

No more square boxes on this page in Chrome now :)
At the meeting yesterday, we discussed putting this on /start/, too, since a number of people leave it set as their home page, and generally were in favor of it.

What I didn't think about at the time is that /start/ would have two messages if people were using old versions/builds and old Flash, which seems like it'd be obnoxious and overwhelming.  I think to put this on /start/, we'd have to condition it on only displaying if the out-of-date message wasn't also showing.
Here's the script that's in use on the test page, i.e. with my hacks to it included.
Assignee: samuel.sidler → alqahira
Attachment #472441 - Attachment is obsolete: true
Attachment #474431 - Flags: review?(samuel.sidler)
Attachment #472441 - Flags: review?(samuel.sidler)
(In reply to comment #16)
> The script assumes that there will be a node (I envision an empty div) with id
> "flashwarning" on the page.

This approach seems fine.

> If people don't like that approach, I'm happy to change it to whatever anyone
> wants. I can also adjust structure as necessary (e.g., if we need the text in a
> <p> in the node instead of directly inside, for styling purposes).

Do we technically need to put text in a <p> to keep it HTML5 compliant? I don't remember the rules (or lack thereof) for HTML5.

> - Only supports OS X, but should work in other browsers (I've tested Camino 1.6
> and 2.1a1pre, Chrome 6, and Firefox 3.6).

Let's not worry about browsers other than Camino since this page is meant to be opened in Camino.

> - I have no idea if we have a JS style for our site, so I just did what I'm
> used to. I can change if we do have a style.

We don't really have a specific style. What you have here is fine and I'm reviewing it now.
Attachment #474431 - Attachment mime type: application/x-javascript → text/plain
Comment on attachment 474431 [details]
Stuart's detection/warning script, as hacked by me

>  var explanation = document.createTextNode(
>      'You are running an out-of-date version of Flash Player, which may ' +
>      'have security vulnerabilities or stability issues. We strongly ' +
>      'recommend that you '); 
>  warningText.appendChild(explanation);
>  
>  var updateLink = document.createElement('a');
>  updateLink.textContent = 'update to the latest version';
>  if (canRunLatestFlash()) {
>    updateLink.setAttribute('href', 'http://get.adobe.com/flashplayer/');
>  } else {
>    updateLink.setAttribute(
>        'href', 'http://get.adobe.com/flashplayer/otherversions/');
>  }

I'm not a fan of the way we do this. I'd like it much better if explanation was part of the canRunLatestFlash if statement and we simply repeated it. Adding links in this manner just feels a little sketchy, though I know why you did it.

That said, I don't care all that much, so keep it as is if that's how most JS would write it. The most we should ever have to do here is edit some text and updates the links and that's simple no matter how this is written.

r=me.
Attachment #474431 - Flags: review?(samuel.sidler) → review+
(I didn't actually tick the box to take the bug.)
Assignee: alqahira → stuart.morgan+bugzilla
(In reply to comment #28)
> Adding links in this manner just feels a little sketchy, though I know why you did it.

I'm not sure what you mean; in the DOM the paragraph is in fact thee distinct nodes (text, link, text). Are you suggesting innerHTML as the less hacky solution? If so I would disagree.

> Do we technically need to put text in a <p> to keep it HTML5 compliant?

Div can contain both block-level and inline children, so it's not necessary, but once we have the h2 also I think it's cleaner to have the p also. Luckily Smokey added it for me :)
We're stalled here on coming up with text for the <h2>.

Sam noted that we can't really use any text like "You've updated Camino; now update your Flash" because /welcome/ is seen both by new installs and updates.

We could maybe get around that by saying "You have a new Camino" (which should be true both for new users and for updates), but that's dangerously close to implying that it's a current/up-to-date Camino, which it may not be.

"For best results with Camino, please update Flash" ?
[11:29pm] ss: "Camino's awesome, but for best results, please update Flash"
Why do we need to mention Camino? We could just say "Flash needs to be updated".
For the first page a user sees when installing Camino? Seems a bit rough.
"Camino needs your help to keep you safe on the internet"?
That's... close. Maybe it's just because I know Camino is open source, but it makes it sound too much like we're asking for volunteers.

"Camino is almost ready to use"
"Now that you have Camino, update Flash to stay safe"
"Camino is free, so is the latest Flash update"

There's more bad where those came from... believe me.
(In reply to comment #36)

> "Now that you have Camino, update Flash to stay safe"

I find that the least unpalatable of Sam's 3.  Stuart's seems a bit long, in addition to volunteer-y.

"Keep Camino running well by updating Flash" :P
The problem with mentioning Camino at all is that it sounds like it's something wrong with us, rather than being equally true of all the other browsers (that don't bundle Flash themselves to avoid this problem).

"For the best browsing experience, update Flash"?
[5:33pm] ss: "Update Flash to keep Camino fast and safe"

I have to agree with comment 38, although I do like Sam's suggestion from irc.

How about this?  We go with Stuart's suggestion in comment 38 for the <h2>, but we rework the message text a little bit to make it a kinder experience "[f]or the first page a user sees when installing Camino"?

So:

<h2>For the best browsing experience, update Flash</h2>

<p>Camino has detected that you are running an out-of-date version of Flash Player, which may have security vulnerabilities or stability issues.  Like all web browsers, Camino functions best when plug-ins such as Flash are up-to-date. We strongly recommend that you <a href="">update to the latest version of Flash</a>.</p>

(by comparison, the current text: 
<h2>We have no agreement on text to use here</h2>
<p>You are running an out-of-date version of Flash Player, which may have security vulnerabilities or stability issues. We strongly recommend that you <a href="">update to the latest version</a>.</p>)

It's a bit longer than what's currently staged, but it's "friendlier", yet, I hope, doesn't compromise our message (we could s/strongly recommend that you/[strongly] urge you to/, too, if desired).
Comment 39 is fine by me, but I'd change "which may have security vulnerabilities or stability issues" to "which has known security vulnerabilities and stability issues." We may not know for sure about the security problems, but we're sure each version of Flash has stability issues.
Can we cut the "Camino has detected that" part? That would make it shorter, and that part doesn't seem to add anything (also, it's not exactly accurate, and implies that Camino will always let them know when Flash is out of date). Otherwise it sounds good to me (with or without comment 40; I don't have a preference).
Deployed on /welcome/ with the text in comment 39, less "Camino has detected".

Comment 40 actually says the reverse of what it intends to say, and, even then, we can't always guarantee that a given old version Flash is going to have stability issues.  It's not our product, and I'd prefer to tread more carefully with what we claim about it.

Bug 596135 for arguing over /start/

Thanks everyone for making this happen!
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Flags: camino2.0.5? → camino2.0.5+
w00t!
So, this is really going to suck for users due to bug 313700 :( (it hit me tonight, and made me question my sanity, or my ability to do math).
Only for people who have session restore though, right? Otherwise /welcome/ won't re-open.
To summarize from irc: for whatever reason, sometimes pluginreg.dat just never updates.  (In my case, I had physically removed the old Flash and installed the new one, so no matter what the "do-i-rebuild?" code was checking, it should have seen new timestamps.)  In this case, the bad data could persist through the next Camino update, which would cause the user to see an incorrect warning. 

Stuart suggested we could touch a bundle wrapper directory on quit for an autoupdate (but that wouldn't solve my problem), or nuke pluginreg.dat before autoupdates (which would be a bit sketchy, but maybe better than nothing).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: