Closed
Bug 688925
Opened 13 years ago
Closed 13 years ago
Use unique links for the plugin-crashed UI (notification bar vs in-content)
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 10
Tracking | Status | |
---|---|---|
firefox9 | --- | fixed |
People
(Reporter: Dolske, Assigned: felix)
References
Details
(Keywords: user-doc-needed, Whiteboard: [qa?])
Attachments
(2 files)
2.40 KB,
patch
|
Dolske
:
review+
asa
:
approval-mozilla-aurora+
asa
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
1.52 KB,
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•13 years ago
|
||
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?
Assignee | ||
Comment 2•13 years ago
|
||
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.
Attachment #562220 -
Flags: review?(dolske)
Comment 3•13 years ago
|
||
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!
Updated•13 years ago
|
Keywords: user-doc-needed
Comment 4•13 years ago
|
||
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.
Reporter | ||
Comment 5•13 years ago
|
||
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)
Attachment #562220 -
Flags: review?(dolske) → review+
Reporter | ||
Comment 6•13 years ago
|
||
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.
Attachment #562220 -
Flags: approval-mozilla-beta?
Attachment #562220 -
Flags: approval-mozilla-aurora?
Comment 7•13 years ago
|
||
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.
Attachment #562220 -
Flags: approval-mozilla-beta?
Attachment #562220 -
Flags: approval-mozilla-beta-
Attachment #562220 -
Flags: approval-mozilla-aurora?
Attachment #562220 -
Flags: approval-mozilla-aurora+
Comment 8•13 years ago
|
||
(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.
Keywords: privacy-review-needed
Comment 9•13 years ago
|
||
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.
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 10•13 years ago
|
||
This the changeset a la comment 5. I cloned the aurora repo and tested against it.
Comment 11•13 years ago
|
||
Pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/4027f8761df6
Comment 12•13 years ago
|
||
What, if anything, can QA do to verify this once it is fixed?
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-team][qa?]
Comment 13•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][qa?] → [qa?]
Target Milestone: --- → Firefox 9
Comment 14•13 years ago
|
||
Oops, got confused about target milestone. Hopefully I did this correctly now!
status-firefox9:
--- → fixed
Target Milestone: Firefox 9 → Firefox 10
Comment 15•13 years ago
|
||
[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!
Assignee | ||
Comment 16•13 years ago
|
||
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•13 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•