Closed
Bug 847072
Opened 12 years ago
Closed 10 years ago
Plugin deprecation warning cuts off Plugin name
Categories
(Core Graveyard :: Plug-ins, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla35
People
(Reporter: b.eckenfels, Assigned: bruteforks, Mentored)
References
Details
(Whiteboard: [lang=JS][good first bug])
Attachments
(2 files, 3 obsolete files)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0 Build ID: 20130215130331 Steps to reproduce: Opened a webn site which has a Java Applet. This is on Windows 7, with a fairly standard resolution/font ratio. Actual results: Firefox told me that the Java Version is vulnerable and asked me if I want to activate it. The warning message had cut off the name of the plugin right before the actual interesting Version number. Expected results: Show the whole Plugin name, at least for most common deprecation warning for the Java Plugin.
Reporter | ||
Comment 1•12 years ago
|
||
The "longest" Plugin Name I have on my System is "HP Virtual Room Client Launcher Plugin" and the Java Plugins are named: Java(TM) Platform SE 7 U15 File: npjp2.dll Version: 10.15.2.3 Next Generation Java Plug-in 10.15.2 for Mozilla browsers
Updated•12 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•12 years ago
|
||
The click-to-play UI is currently getting a redesign / new concept, so as this is not a pressing issue we should see whether this is obsoleted by UI changes. If it isn't i think we should do a proper "..." cutoff and maybe show the full string as a tooltip or something.
OS: Windows 7 → All
Priority: -- → P3
Hardware: x86_64 → All
As it was originally designed, that UI intentionally removes (or at least tries to remove) version numbers from the plugin name (see https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-plugins.js#79 ). In the case of plugins that have a secure version to update to, there would be a link to the Mozilla plugin check/update page, so the version number isn't important as long as you replace the one you have with a secure version. However, in this case there isn't a secure version to update to. I'm not actually sure exposing the version number in this UI is very helpful, either, though, because there's really nothing you can do about it (and as an aside, you can see what version you have in about:addons -> Plugins or about:plugins, but I suppose you know that already by your second comment). There may still be a bug with long plugin names, though. Bernd - what happens if you set plugins.click_to_play to true in about:config and then visit a page that uses the plugin with the long name? (or is it not an in-content plugin?)
Flags: needinfo?(b.eckenfels)
Comment 4•12 years ago
|
||
(In reply to David Keeler (:keeler) from comment #3) > what > happens if you set plugins.click_to_play to true in about:config and then > visit a page that uses the plugin with the long name? Same behavior
Comment 5•12 years ago
|
||
David, so this is just the version number trimming cutting off the 15? Couldn't this just simply special-case Java then?
Updated•12 years ago
|
Flags: needinfo?(b.eckenfels)
(In reply to Georg Fritzsche [:gfritzsche] from comment #5) > David, so this is just the version number trimming cutting off the 15? As far as I can tell, yes (and like I said, this is the intended behavior). > Couldn't this just simply special-case Java then? Special-case to not trim the version number for Java? Or special-case to take off the "U" as well, since it's confusing that it gets left behind?
Reporter | ||
Comment 7•12 years ago
|
||
I guess it does not make sense to argue about this "hide all usefull technical information" policy in Firefox. However I tell you anyway why I think it is important to know what Java Version is blocked: Oracle releases a new Version now every week and Mozilla now deprecvates and blocks versions every week. The dialog should tell me (without any further actions) if the problem was my Plugin Version or Oracle. (in this case it was Oracle). How should I make an informed decision of allowing this Plugin, if I dont know what Version I will allow?
Comment 8•12 years ago
|
||
(In reply to David Keeler (:keeler) from comment #6) > Or special-case to > take off the "U" as well, since it's confusing that it gets left behind? That, sounds simple and should also apply regardless of redesigns. Or, what do you think?
Comment 9•11 years ago
|
||
With the reimplemented doorhanger (bug 880735 and dependencies) we just show "Java Applet".
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 10•11 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #9) > With the reimplemented doorhanger (bug 880735 and dependencies) we just show > "Java Applet". I disagree, on Firefox 26 I still see a "enable plugin" Warning which says "Activate Java(TM) Platform SE 7 U.". Even on the "FAQ" pages screenshots the broken version number is visible: https://support.mozilla.org/en-US/kb/why-do-i-have-click-activate-plugins?as=u&utm_source=inproduct
Comment 11•11 years ago
|
||
Thanks for the heads-up - it's currently fine on OS X ("Java Applet Plug-in"), but not on Windows ("Java(TM) Plaftform SE 7 UNN"). This is due to how makeNicePluginName() [1] works. We really should just special-case for Java here. [1] http://hg.mozilla.org/mozilla-central/annotate/1ad9af3a2ab8/browser/base/content/browser-plugins.js#l71
Assignee: nobody → georg.fritzsche
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment 12•11 years ago
|
||
For this bug we need to change makeNicePluginName(). I think we should be fine if we return "Java" for |aName| starting with "Java" and followed by a non-letter (to cover "Java(TM)" etc.).
Assignee: georg.fritzsche → nobody
Whiteboard: [mentor=gfritzsche][lang=JS][good first bug]
Comment 13•11 years ago
|
||
Hi, I am interested in working on this bug. So please can u assign this bug to me? Thanks in Advance, Regards, A. Anup
Comment 14•11 years ago
|
||
Sure, but lets check this out after bug 863773.
Updated•10 years ago
|
Mentor: georg.fritzsche
Whiteboard: [mentor=gfritzsche][lang=JS][good first bug] → [lang=JS][good first bug]
Comment 15•10 years ago
|
||
Note that makeNicePluginName() moved to PluginContent.jsm due to the recent e10s work: http://hg.mozilla.org/mozilla-central/annotate/5e704397529b/browser/modules/PluginContent.jsm#l141
Updated•10 years ago
|
Assignee: nobody → bruteforks
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #12) > For this bug we need to change makeNicePluginName(). > I think we should be fine if we return "Java" for |aName| starting with > "Java" and followed by a non-letter (to cover "Java(TM)" etc.). I added a regex to check for "Java" at the beginning of the string, followed by a non-letter character. It seems to work. Is this the correct approach?
Comment 17•10 years ago
|
||
Comment on attachment 8497383 [details] [diff] [review] bug847072.patch Review of attachment 8497383 [details] [diff] [review]: ----------------------------------------------------------------- Note that you can request feedback on a patch from specific people by setting the feedback flag to ? and picking the requestee. ::: browser/modules/PluginContent.jsm @@ +140,5 @@ > // Map the plugin's name to a filtered version more suitable for user UI. > makeNicePluginName : function (aName) { > if (aName == "Shockwave Flash") > return "Adobe Flash"; > + else if (/^Java[^\w]/.exec(aName)) We don't need an |else| here, we already |return| for the Flash case, otherwise this looks fine.
Assignee | ||
Comment 18•10 years ago
|
||
Ok, updated. (In reply to Georg Fritzsche [:gfritzsche] from comment #17) > Note that you can request feedback on a patch from specific people by > setting the feedback flag to ? and picking the requestee. Ah, thanks for the tip.
Attachment #8497383 -
Attachment is obsolete: true
Attachment #8497468 -
Flags: feedback?(georg.fritzsche)
Comment 19•10 years ago
|
||
Comment on attachment 8497468 [details] [diff] [review] bug847072v1.1.patch Review of attachment 8497468 [details] [diff] [review]: ----------------------------------------------------------------- This still looks good, i think we should look into test coverage next.
Attachment #8497468 -
Flags: feedback?(georg.fritzsche) → feedback+
Comment 20•10 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #19) > This still looks good, i think we should look into test coverage next. Actually, test-coverage for the string-building for the notification gets a little involved. We don't have coverage for other notification strings and - sadly - no nice setup to unit-test things like makeNicePluginName(). I think we should skip it in this specific case.
Updated•10 years ago
|
Attachment #8497468 -
Flags: review?(mconley)
Reporter | ||
Comment 21•10 years ago
|
||
Just for the record, it does not look good to me. It does not make sense to ask a user if he wants to activate a plugin and not tell him what version it is :-/
Comment 22•10 years ago
|
||
This didn't change here though, this just fixes the broken name handling for Java on Windows. The additional version information is probably a discussion for firefox-dev or maybe an enhancement bug.
Comment 23•10 years ago
|
||
Comment on attachment 8497468 [details] [diff] [review] bug847072v1.1.patch Review of attachment 8497468 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the patch! Just two suggestions. ::: browser/modules/PluginContent.jsm @@ +140,5 @@ > // Map the plugin's name to a filtered version more suitable for user UI. > makeNicePluginName : function (aName) { > if (aName == "Shockwave Flash") > return "Adobe Flash"; > + if (/^Java[^\w]/.exec(aName)) I believe [^\w] is equivalent to \W. Also, a comment about what this regex is doing would be good for future readers.
Attachment #8497468 -
Flags: review?(mconley)
Reporter | ||
Comment 24•10 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #22) > This didn't change here though, this just fixes the broken name handling for > Java on Windows. > The additional version information is probably a discussion for firefox-dev > or maybe an enhancement bug. yes maybe, however the Comment#0 (original bug report) even had a story to explain why the version is needed and the result was it was removed :/
Assignee | ||
Comment 25•10 years ago
|
||
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #23) > I believe [^\w] is equivalent to \W. > > Also, a comment about what this regex is doing would be good for future > readers. Thanks, I made the suggested changes.
Attachment #8497468 -
Attachment is obsolete: true
Comment 26•10 years ago
|
||
Comment on attachment 8497674 [details] [diff] [review] bug847072v1.2.patch Review of attachment 8497674 [details] [diff] [review]: ----------------------------------------------------------------- Looks good - thanks!
Attachment #8497674 -
Flags: review+
Comment 27•10 years ago
|
||
Trivial change and fine locally, a try run is not needed here.
Keywords: checkin-needed
Assignee | ||
Comment 28•10 years ago
|
||
Just added mconley to reviewer field of commit message
Attachment #8497674 -
Attachment is obsolete: true
Comment 29•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca5765dddaed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ca5765dddaed
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 31•10 years ago
|
||
A quick check on Windows 7 x64 using Firefox 35 Beta 3 (BuildID=20141211142524) confirms that "Java" is now displayed in the doorhanger instead of "Java Platform SE 7 U" (which displays in Firefox 34.0.5). This was the intended fix so marking this as Verified.
Status: RESOLVED → VERIFIED
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•