261790 - port various changes betwen xpfe/toolkit

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

I diffed toolkit against xpfe/global... there were various unported changes that
can be trivially ported.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[Mike Connor [:mconnor]]

Comment on attachment 160232 [details] [diff] [review]
patch

>+++ xpfe/global/resources/content/bindings/dialog.xml	27 Sep 2004 14:43:36 -0000
>@@ -193,44 +193,44 @@
>             buttons[dlgtype] = exBtns[i];
>           }
> 
>           // add the label and oncommand handler to each button
>           for (dlgtype in buttons) {
>             var button = buttons[dlgtype];
>-            buttons[dlgtype].addEventListener("command", this._handleButtonCommand, true);
>+            button.addEventListener("command", this._handleButtonCommand, true);
>+
>+

kill the extra line here

>-            for (dlgtype in shown) {
>-              if (shown[dlgtype])
>-                buttons[dlgtype].hidden = false;
>-              else
>-                buttons[dlgtype].hidden = true;
>-            }
>+            for (dlgtype in buttons) 
>+              buttons[dlgtype].hidden = !shown[dlgtype];
>+
>+

and here, too, and I'd imagine in the toolkit version, if you would

Comment 3

[[e-mail]]

Comment on attachment 160232 [details] [diff] [review]
patch

>     dump("An error occurred executing the "+command+" command\n");
>+    dump(e+"\n")
Might as well concatenate this into one big string, with spaces around +

>+/* hide the content, but don't destroy the frames.  Make sure this
>+comes before any rules involving "hidden"  */
>+*[collapsed="true"], *[moz-collapsed="true"] {
>+  visibility: collapse;
>+}
>+
>+/* The rule for hidden comes AFTER the rule for collapsed.  That way if
>+both are specified on an element, the hidden rule wins. */
> /* hide the content and destroy the frame */
> *[hidden="true"] {
>   display: none;
> }
> 
>-/* hide the content, but don't destroy the frames */
>-*[collapsed="true"], 
>-*[moz-collapsed="true"] {
>-  visibility: collapse;
>-}
>-
>-
Please ask someone if this makes any sense, it makes no sense to me.

>+            try {
>+              return this.stringBundle.formatStringFromName(aStringKey, aStringsArray, aStringsArray.length);
>+            }
>+            catch (e) {
>+              dump("*** Failed to get string " + aStringKey + " in bundle: " + this.src + "\n");
>+              throw e;
>+            }
More accurately the string bundle failed to format the string...

>+            const nsIContentPolicy = Components.interfaces.nsIContentPolicy;
>             try {
>               var contentPolicy =
>                 Components.classes['@mozilla.org/layout/content-policy;1']
>-                          .getService(Components.interfaces.nsIContentPolicy);
>+                          .getService(nsIContentPolicy);
Need to change the other four uses too, surely?

>+            var blank = (aURI == "about:blank");
>+
>             if (!this.mTabbedMode)
>               this.enterTabbedMode();
> 
>             var t = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul",
>                                              "tab");
> 
>-            var blank = (aURI == "about:blank");
>-
>             if (blank)
Personally I think declaring the variable near its use makes most sense.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

> and here, too, and I'd imagine in the toolkit version, if you would

done. second toolkit instance has some more code, so I left it.

> Please ask someone if this makes any sense, it makes no sense to me.

hm, yeah. I got confirmation that it makes no sense. will revert this change,
and copy to the ffox version.

> More accurately the string bundle failed to format the string...

will change it to "Failed to format string"

> Need to change the other four uses too, surely?

hm, yeah. ffox's code is quite a bit different here.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

toolkit parts need relanding

Comment 8

[Ian Neal]

Relevant parts of patch relanded after aviary branch landing