Open
Bug 660732
Opened 14 years ago
Updated 3 years ago
about:support "Try updating your graphics driver" should link to instructions
Categories
(Toolkit :: General, enhancement)
Toolkit
General
Tracking
()
REOPENED
Firefox 7
People
(Reporter: jruderman, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
5.41 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
about:support shows me:
"GPU Accelerated Windows: 0/1. Blocked for your graphics driver version. Try updating your graphics driver to version NVIDIA 257.21 or newer."
"Updating your graphics driver" should be a link to https://support.mozilla.com/en-US/kb/how-do-i-upgrade-my-graphics-drivers. Having to figure it out on your own is not fun.
Comment 1•14 years ago
|
||
Comment 2•14 years ago
|
||
Comment on attachment 536502 [details] [diff] [review]
Patch v1
>diff --git a/toolkit/content/aboutSupport.js b/toolkit/content/aboutSupport.js
>+ try {
>+ let urlFormatter = Cc["@mozilla.org/toolkit/URLFormatterService;1"]
I don't think any of this is expected to throw, so you should remove the try/catch.
>+ for (let i = 0; i < links.length; i++) {
>+ links[i].href = supportUrl;
Might be nice to also set target="_blank"
>-function createElement(tagName, textContent, opt_class) {
>+function createElement(tagName, content, opt_class, isHTML) {
Why not just set innerHTML instead of textContent unconditionally? It's possible for errorMessageForFeature output to be used for something other than pushFeatureInfoRow, so even if this particular string can't currently show up there in practice, it seems a bit fragile.
Attachment #536502 -
Flags: review?(gavin.sharp) → review-
Comment 3•14 years ago
|
||
(In reply to comment #2)
> >+ try {
> >+ let urlFormatter = Cc["@mozilla.org/toolkit/URLFormatterService;1"]
>
> I don't think any of this is expected to throw, so you should remove the
> try/catch.
For some reason I thought formatURLPref() threw on a missing pref - but it just returns "about:blank". So I've replaced the try/catch with a check for about:blank, in case a toolkit app doesn't set the pref (the support page is in toolkit, and the pref is set per-application).
Attachment #536502 -
Attachment is obsolete: true
Attachment #537678 -
Flags: review?(gavin.sharp)
Comment 4•14 years ago
|
||
Comment on attachment 537678 [details] [diff] [review]
Patch v2
Little bit afraid that the "don't translate" note will be missed, but that just causes no link to appear, which is pretty good fallback behavior!
Attachment #537678 -
Flags: review?(gavin.sharp) → review+
Comment 5•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Flags: in-litmus?
OS: Linux → All
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → Firefox 7
Comment 6•14 years ago
|
||
This patch totally broke about:support
Extensions, Modified Prefs and Graphics are all empty only the banner bar for each category shows.
These errors in Console2
Error: not well-formed
Source code:
<td xmlns="http://www.w3.org/1999/xhtml">&PT</
Error: An invalid or illegal string was specified = NS_ERROR_DOM_SYNTAX_ERR
Source file: chrome://global/content/aboutSupport.js
Line: 382
aka/Littlemutt
Comment 8•14 years ago
|
||
I've backed this out because of bug 662901. Would love to see at least a basic test that about:support loads here before landing.
http://hg.mozilla.org/mozilla-central/rev/84189d94f01a
Status: RESOLVED → REOPENED
Flags: in-testsuite- → in-testsuite?
Resolution: FIXED → ---
Updated•13 years ago
|
Assignee: bmcbride → nobody
Flags: in-testsuite?
Flags: in-litmus?
Updated•12 years ago
|
Product: Firefox → Toolkit
Updated•3 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•