Closed Bug 667201 Opened 13 years ago Closed 13 years ago

Front end changes for Bug 545070: plugin-problem UI shouldn't say "click here".

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(seamonkey2.3 fixed, seamonkey2.4 fixed)

RESOLVED FIXED
seamonkey2.5
Tracking Status
seamonkey2.3 --- fixed
seamonkey2.4 --- fixed

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch Patch v1.0 (obsolete) (deleted) — Splinter Review
Adapt the changes from Bug 545070 > diff --git a/suite/themes/modern/mozapps/plugins/contentPluginMissing.png b/suite/themes/modern/mozapps/plugins/contentPluginMissing.png > new file mode 100644 pngcrush -rem gAMA -rem cHRM -rem iCCP -rem sRGB contentPluginMissing.png contentPluginMissing-1.png Didn't do anything. Experimenting with other switches made the resulting file larger. optipng -zc1-9 -zm1-9 -zs0-3 -f0-5 -nx contentPluginMissing.png Complained that the file is already optimized.
Attachment #541937 - Flags: review?(neil)
Comment on attachment 541937 [details] [diff] [review] Patch v1.0 >+ var plugin = aEvent.target; Nit: two spaces before = >+ // Hide the in-content UI if it's too big. The crashed plugin handler already does this. On the other hand, the crashed plugin handler doesn't show the infobar in this case... >+ <method name="installSinglePlugin"> But you actually end up installing all the missing plugins... >+ var doc = plugin.ownerDocument;; Nit: double semicolon >+ var installStatus = doc.getAnonymousElementByAttribute(plugin, "class", "installStatus"); >+ installStatus.setAttribute("status", "ready"); >+ var iconStatus = doc.getAnonymousElementByAttribute(plugin, "class", "icon"); >+ iconStatus.setAttribute("status", "ready"); So, what's the point of this?
Attached patch Patch v1.1 (obsolete) (deleted) — Splinter Review
> neil@parkwaycc.co.uk 2011-06-27 16:45:55 PDT > > Comment on attachment 541937 [details] [diff] [review] [diff] [details] [review] > Patch v1.0 > >>+ var plugin = aEvent.target; > Nit: two spaces before = Fixed. >>+ // Hide the in-content UI if it's too big. The crashed plugin handler already does this. > On the other hand, the crashed plugin handler doesn't show the infobar in this case... I see code in PluginCrashed() that calls showPluginCrashedNotification() if the in-content UI overflows the plugin container. So I don't know what you want me to do here. >>+ <method name="installSinglePlugin"> > But you actually end up installing all the missing plugins... I've changed method name to "installMissingPlugins". Note despite installing all missing plugins, Firefox calls their function installSinglePlugin(). >>+ var doc = plugin.ownerDocument;; > Nit: double semicolon Fixed. >>+ var installStatus = doc.getAnonymousElementByAttribute(plugin, "class", "installStatus"); >>+ installStatus.setAttribute("status", "ready"); >>+ var iconStatus = doc.getAnonymousElementByAttribute(plugin, "class", "icon"); >>+ iconStatus.setAttribute("status", "ready"); > So, what's the point of this? From https://bugzilla.mozilla.org/show_bug.cgi?id=545070#c6 > * for plugin-not-found <embed>s, set the new "status" attribute to enable > display of the "click to install" text. [As noted above, this is the first > step towards giving it the smarts to check for something being available > _before_ inviting the user to click.] So somewhere in /toolkit/mozapps/plugins/content/pluginProblemContent.css we have: http://hg.mozilla.org/mozilla-central/rev/6c3b228838e5#l7.39 > 7.39 +.installStatus[status="ready"] .msgInstallPlugin { > 7.40 + display: block; > 7.41 +} > 7.42 + And in stripe we have: > 11.14 +:-moz-type-unsupported .icon[status="ready"] { > 11.15 background-image: url(chrome://mozapps/skin/plugins/contentPluginDownload.png); > 11.16 }
Attachment #541937 - Attachment is obsolete: true
Attachment #549370 - Flags: review?(neil)
Attachment #541937 - Flags: review?(neil)
Attachment #549370 - Attachment is patch: true
(In reply to comment #3) >(In reply to comment #2) >> (From update of attachment 541937 [details] [diff] [review]) >>>+ // Hide the in-content UI if it's too big. The crashed plugin handler already does this. >> On the other hand, the crashed plugin handler doesn't show the infobar in this case... >I see code in PluginCrashed() that calls showPluginCrashedNotification() if >the in-content UI overflows the plugin container. So I don't know what you >want me to do here. Well, I guess we could leave it for now, the plugin crashed notification is slightly different for other reasons anyway. >>>+ <method name="installSinglePlugin"> >> But you actually end up installing all the missing plugins... >I've changed method name to "installMissingPlugins". Note despite installing >all missing plugins, Firefox calls their function installSinglePlugin(). Yeah, well that's Firefox for you... >>>+ var installStatus = doc.getAnonymousElementByAttribute(plugin, "class", "installStatus"); >>>+ installStatus.setAttribute("status", "ready"); >>>+ var iconStatus = doc.getAnonymousElementByAttribute(plugin, "class", "icon"); >>>+ iconStatus.setAttribute("status", "ready"); >> So, what's the point of this? >From https://bugzilla.mozilla.org/show_bug.cgi?id=545070#c6 Fair enough.
Comment on attachment 549370 [details] [diff] [review] Patch v1.1 >+ if (this.isTooSmall(plugin, overlay)) >+ overlay.style.visibility = "hidden"; Nit: wrong indentation >+ <method name="installMissingPlugins"> > <parameter name="aEvent"/> Nit: don't need aEvent any more, addLinkClickCallback handles it. >+ this.addLinkClickCallback(installLink, installMissingPlugins, event); As above. >+ callback: this.installMissingPlugins.bind(this, event) As above. >diff --git a/suite/themes/modern/mozapps/plugins/contentPluginMissing.png b/suite/themes/modern/mozapps/plugins/contentPluginMissing.png >new file mode 100644 Nit: needs to be run through optipng
Attachment #549370 - Flags: review?(neil) → review+
Attached patch Patch v1.2 for checkin. r=Neil (deleted) — Splinter Review
> Comment on attachment 549370 [details] [diff] [review] [diff] [details] [review] > Patch v1.1 > >>+ if (this.isTooSmall(plugin, overlay)) >>+ overlay.style.visibility = "hidden"; > Nit: wrong indentation Fixed. >>+ <method name="installMissingPlugins"> >> <parameter name="aEvent"/> > Nit: don't need aEvent any more, addLinkClickCallback handles it. Fixed. >>+ this.addLinkClickCallback(installLink, installMissingPlugins, event); > As above. Fixed. >>+ callback: this.installMissingPlugins.bind(this, event) > As above. Fixed. >>diff --git a/suite/themes/modern/mozapps/plugins/contentPluginMissing.png b/suite/themes/modern/mozapps/plugins/contentPluginMissing.png >>new file mode 100644 > Nit: needs to be run through optipng Already done in Comment 1 !!
Attachment #549370 - Attachment is obsolete: true
Attachment #549558 - Flags: review+
Comment on attachment 549558 [details] [diff] [review] Patch v1.2 for checkin. r=Neil Target Milestone for the mozilla-central Bug 545070 is mozilla6. Requesting approval for comm-aurora and comm-beta. No L10N changes.
Attachment #549558 - Flags: approval-comm-beta?
Attachment #549558 - Flags: approval-comm-aurora?
(In reply to comment #7) > Pushed to comm-central > http://hg.mozilla.org/comm-central/rev/b0be62c945b9 http://hg.mozilla.org/comm-central/rev/b0be62c945b9#l1.167 You might want to sort the unneeded event argument.
Comment on attachment 549558 [details] [diff] [review] Patch v1.2 for checkin. r=Neil a=me for comm-aurora/beta with extraneous event argument removed.
Attachment #549558 - Flags: approval-comm-beta?
Attachment #549558 - Flags: approval-comm-beta+
Attachment #549558 - Flags: approval-comm-aurora?
Attachment #549558 - Flags: approval-comm-aurora+
> http://hg.mozilla.org/comm-central/rev/b0be62c945b9#l1.167 > You might want to sort the unneeded event argument. Puushed a fix r=typo http://hg.mozilla.org/comm-central/rev/423a04323c61
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.5
Blocks: 724499
Test will be updated by bug 724499.
Depends on: 545070
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: