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)
SeaMonkey
UI Design
Tracking
(seamonkey2.3 fixed, seamonkey2.4 fixed)
RESOLVED
FIXED
seamonkey2.5
People
(Reporter: philip.chee, Assigned: philip.chee)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
philip.chee
:
review+
iannbugzilla
:
approval-comm-aurora+
iannbugzilla
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•13 years ago
|
||
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 2•13 years ago
|
||
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?
Assignee | ||
Comment 3•13 years ago
|
||
> 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)
Updated•13 years ago
|
Attachment #549370 -
Attachment is patch: true
Comment 4•13 years ago
|
||
(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 5•13 years ago
|
||
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+
Assignee | ||
Comment 6•13 years ago
|
||
> 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+
Assignee | ||
Comment 7•13 years ago
|
||
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/b0be62c945b9
Assignee | ||
Comment 8•13 years ago
|
||
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 10•13 years ago
|
||
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+
Assignee | ||
Comment 11•13 years ago
|
||
> 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
Assignee | ||
Comment 12•13 years ago
|
||
Pushed:
http://hg.mozilla.org/releases/comm-aurora/rev/e8172bb6b4f1
http://hg.mozilla.org/releases/comm-beta/rev/2e92298d561c
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
status-seamonkey2.3:
--- → fixed
status-seamonkey2.4:
--- → fixed
Target Milestone: --- → seamonkey2.5
You need to log in
before you can comment on or make changes to this bug.
Description
•