The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in seamonkey2.5

Status

SeaMonkey
UI Design
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Philip Chee, Assigned: Philip Chee)

Tracking

Trunk
seamonkey2.5
Dependency tree / graph
Bug Flags:
in-testsuite -

SeaMonkey Tracking Flags

(seamonkey2.3 fixed, seamonkey2.4 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 541937 [details] [diff] [review]
Patch v1.0

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

6 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

6 years ago
Created attachment 549370 [details] [diff] [review]
Patch v1.1

> 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

6 years ago
Attachment #549370 - Attachment is patch: true

Comment 4

6 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

6 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

6 years ago
Created attachment 549558 [details] [diff] [review]
Patch v1.2 for checkin. r=Neil

> 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

6 years ago
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/b0be62c945b9
(Assignee)

Comment 8

6 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?

Comment 9

6 years ago
(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

6 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

6 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

6 years ago
Pushed:
http://hg.mozilla.org/releases/comm-aurora/rev/e8172bb6b4f1
http://hg.mozilla.org/releases/comm-beta/rev/2e92298d561c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
status-seamonkey2.3: --- → fixed
status-seamonkey2.4: --- → fixed
Target Milestone: --- → seamonkey2.5
(Assignee)

Updated

5 years ago
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.