Last Comment Bug 688925 - Use unique links for the plugin-crashed UI (notification bar vs in-content)
: Use unique links for the plugin-crashed UI (notification bar vs in-content)
Status: RESOLVED FIXED
[qa?]
: user-doc-needed
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 10
Assigned To: Felix Fung (:felix)
:
:
Mentors:
Depends on:
Blocks: 665196
  Show dependency treegraph
 
Reported: 2011-09-23 17:52 PDT by Justin Dolske [:Dolske]
Modified: 2011-12-14 06:58 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Plugin-Crash: Unique Notification Bar Help Link to Differentiate Source (2.40 KB, patch)
2011-09-23 23:03 PDT, Felix Fung (:felix)
dolske: review+
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta-
Details | Diff | Splinter Review
Unique Notification Bar Help Link [For Aurora] (1.52 KB, patch)
2011-10-25 12:01 PDT, Felix Fung (:felix)
no flags Details | Diff | Splinter Review

Description Justin Dolske [:Dolske] 2011-09-23 17:52:30 PDT
From bug 665196 comment 5:

"Also add a modifier to the help links (in-content UI and notification bar) to help understand which is generating more clicks."

When a plugin crashes we show some in-content UI with (among other things) a help icon. If the plugin's size is too small (or it's not actually in the DOM), we instead show a notification bar with a "Learn More" link.

Support's noticed that we get a lot of hits on the help page these to pieces of UI link to. I think it would he helpful to use different links for each (one of which might actually redirect to the other), so that we can better understand which UI is causing more user confusion.

Bug 665196 is improving the UI, but we should still change the links. And probably do so on Aurora/Beta, so that we can do a before/after comparison.

It's pretty unlikely that this has privacy impact, but I'll flag for a privacy review anyway.
Comment 1 Justin Dolske [:Dolske] 2011-09-23 18:05:03 PDT
We'll need a change on the SUMO side to support this, Laura did the work back in bug 544137 originally, but I'm not sure who's on hook to do that now?

Currently the code calls openHelpLink() with the "plugin-crashed" help topic... I think we just want to add a "plugin-crashed-notificationbar" (bikeshed away on the name ;), which ends up going to the same place. Not sure what SUMO/metrics needs so that we can differentiate between the two in stats?
Comment 2 Felix Fung (:felix) 2011-09-23 23:03:47 PDT
Created attachment 562220 [details] [diff] [review]
Plugin-Crash: Unique Notification Bar Help Link to Differentiate Source

Changes:
- Changed plugin-crashed -> plugin-crashed-notificationbar for link from notification bar.
- Removed get crashReportHelpURL from gPluginHandler (since this won't be _only_ help url anymore and it was only being called in one place, it didn't make sense as a pluginHandler function.)

WARNING: This will cause all help coming from the notification bar to 404 unless the appropriate changes to SUMO are made!

Let's skip the bikeshedding. Let's write the necessary rewrite-cond and then track them metrics.
Comment 3 David Tenser [:djst] 2011-09-29 04:12:31 PDT
Cc'ing Michael Verdi, SUMO's content expert and manager. Michael, can you ensure our work gets done so we don't end up 404:ing? Thanks!
Comment 4 Verdi [:verdi] 2011-09-29 07:56:01 PDT
We added the "plugin-crashed-notificationbar" help topic so the link should work. It looks like Webtrends isn't good about figuring out in-product links. Right now it shows 217,988 visits coming from "Direct Traffic" and in second place http://wer.microsoft.com/ with 39 for the month. We'll have to ask metrics to look at the logs and split that up.
Comment 5 Justin Dolske [:Dolske] 2011-09-30 17:33:13 PDT
Comment on attachment 562220 [details] [diff] [review]
Plugin-Crash: Unique Notification Bar Help Link to Differentiate Source

Review of attachment 562220 [details] [diff] [review]:
-----------------------------------------------------------------

Before landing, please check that the notification bar link is now working.

::: browser/base/content/browser.js
@@ -6664,5 @@
>      Cu.import("resource://gre/modules/CrashSubmit.jsm", this);
>      return this.CrashSubmit;
>    },
>  
> -  get crashReportHelpURL() {

Hmpf. I guess having this as a smart getter wasn't actually all that useful. :)

For landing this onto Aurora/Beta, though, let's leave this getter as-is. Just for the astonishingly unlikely case that some addon might be using this. (Fine as-is for trunk, though)
Comment 6 Justin Dolske [:Dolske] 2011-09-30 17:35:37 PDT
Comment on attachment 562220 [details] [diff] [review]
Plugin-Crash: Unique Notification Bar Help Link to Differentiate Source

Requesting approval for landing on Aurora and Beta.

Extremely low risk (only touches code involved in UI for a crashed plugin, no compat issues with adjustment from last comment). Gives us early data to better understand why the current UI might be confusing users.
Comment 7 Asa Dotzler [:asa] 2011-10-03 15:01:37 PDT
Comment on attachment 562220 [details] [diff] [review]
Plugin-Crash: Unique Notification Bar Help Link to Differentiate Source

seems reasonable to skip right up to aurora but feels too late for beta.
Comment 8 Sid Stamm [:geekboy or :sstamm] 2011-10-19 15:17:04 PDT
(In reply to Justin Dolske [:Dolske] from comment #0)
> It's pretty unlikely that this has privacy impact, but I'll flag for a
> privacy review anyway.

Thanks, Dolske.  You're right, so clearing the flag.
Comment 9 :Margaret Leibovic 2011-10-24 23:05:51 PDT
https://hg.mozilla.org/integration/fx-team/rev/e60d3779a3b8

Felix, for Aurora you should make a patch that addresses what Dolske has to say in comment 5, and then if you upload it back here I can land it on Aurora for you (located at http://hg.mozilla.org/releases/mozilla-aurora). If you want to make sure you're addressing his comment correctly, you could also ask for review again.
Comment 10 Felix Fung (:felix) 2011-10-25 12:01:56 PDT
Created attachment 569447 [details] [diff] [review]
Unique Notification Bar Help Link [For Aurora]

This the changeset a la comment 5. I cloned the aurora repo and tested against it.
Comment 11 :Margaret Leibovic 2011-10-25 17:44:09 PDT
Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/4027f8761df6
Comment 12 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-10-26 09:28:27 PDT
What, if anything, can QA do to verify this once it is fixed?
Comment 13 :Margaret Leibovic 2011-10-27 11:35:09 PDT
https://hg.mozilla.org/mozilla-central/rev/e60d3779a3b8
Comment 14 :Margaret Leibovic 2011-10-27 11:41:50 PDT
Oops, got confused about target milestone. Hopefully I did this correctly now!
Comment 15 Alex Keybl [:akeybl] 2011-12-13 16:10:07 PST
[Triage Comment]
Please provide STR here by tomorrow at 12:00PM PT 12/14. We will be holding our FF9 sign-offs later that afternoon, and need to be able to verify. Thanks!
Comment 16 Felix Fung (:felix) 2011-12-14 06:26:10 PST
STR Fix:

1. Go to the following URL on a test build:
http://dolske.net/mozilla/tests/plugin/crashed_plugin.html
2. Crash the plugin.
3. Click the help link at the bottom left on the plugin crash screen. Note the page and URL.
4. Go to the following URL on a test build:
http://dolske.net/mozilla/tests/plugin/crashed_plugin_small.html
5. Crash the plugin.
6. You should get a notification bar. Click the help link there. Note that the page should be the same as the help page from step 3 but note that the URL should be different.
Comment 17 Verdi [:verdi] 2011-12-14 06:58:21 PST
(In reply to Felix Fung (:felix) from comment #16)

I'm using Firefox 10 with Flash, Quicktime and Silverlight installed but both of those pages tell me I need to install some other, unspecified, plugin. So I'm unable to crash the plugin and click the help links.

Note You need to log in before you can comment on or make changes to this bug.