Closed Bug 261790 Opened 20 years ago Closed 20 years ago

port various changes betwen xpfe/toolkit

Categories

(SeaMonkey :: UI Design, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.8alpha5

People

(Reporter: Biesinger, Assigned: Biesinger)

Details

Attachments

(1 file, 1 obsolete file)

I diffed toolkit against xpfe/global... there were various unported changes that
can be trivially ported.
Attached patch patch (obsolete) — Splinter Review
Attachment #160232 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #160232 - Flags: review?(mconnor)
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8beta
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
Attachment #160232 - Flags: review?(mconnor) → review+
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.
Attachment #160232 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Target Milestone: mozilla1.8beta → mozilla1.8alpha5
> 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.

Attached patch patch v2Splinter Review
Attachment #160232 - Attachment is obsolete: true
checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Product: Core → Mozilla Application Suite
toolkit parts need relanding
Keywords: aviary-landing
Relevant parts of patch relanded after aviary branch landing
Keywords: aviary-landing
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: