Open Bug 1011857 Opened 6 years ago Updated 3 months ago

Implement CustomizableUI for SeaMonkey

Categories

(SeaMonkey :: UI Design, enhancement)

enhancement
Not set

Tracking

(Not tracked)

People

(Reporter: bugzilla, Unassigned)

Details

User Story

(In reply to Michael Buckley from comment #4)
> I have made a started on a module that impliments an interface that is
> almost the same as that of CustomizableUI, which really just serves as a
> wrapper around how the toolbar currently work.  Anyone trying to do a
> restartless addon for SeaMonkey/Thunerbird would have write something like
> this, the only thing I am doing is making it almost the same interface.  
> 
> https://codefisher.org/catch/blog/2015/01/16/customizableui-in-thunderbird-
> and-seamonkey/
> 
> It is working really well in Thunderbird right now, but it is not restoring
> buttons correctly in SeaMonkey.  I hope to get that fixed soonish though.


GitHub link: https://github.com/codefisher/mozbutton_sdk/blob/master/templates/customizable.jsm

Attachments

(3 files, 1 obsolete file)

Now that Firefox has CustomizableUI.jsm available for marshaling the interaction of extensions with the user interface, it would be nice for this to be available for SeaMonkey too.  In principle, it should be able to be implemented in SeaMonkey even though it doesn't have the Australis interface (maybe with one or two of the module's functions as NOPs in SeaMonkey's implementation).  Perhaps a good strategy would be to start with a few basic, core functions, and build up over time to a more complete implementation.  This bug could be a meta-bug for the eventual complete implementation of CustomizableUI in SeaMonkey?
CCing self.
Component: General → UI Design
Version: unspecified → Trunk
It surely would ease add-on interoperability to have this available.
Perhaps the CustomizableUI widgets should be wrapped in a normal toolbar item on a per widget basis so they can live in the Customize Toolbar window and be placed arbitrarily like any classical item.
I have made a started on a module that impliments an interface that is almost the same as that of CustomizableUI, which really just serves as a wrapper around how the toolbar currently work.  Anyone trying to do a restartless addon for SeaMonkey/Thunerbird would have write something like this, the only thing I am doing is making it almost the same interface.  

https://codefisher.org/catch/blog/2015/01/16/customizableui-in-thunderbird-and-seamonkey/

It is working really well in Thunderbird right now, but it is not restoring buttons correctly in SeaMonkey.  I hope to get that fixed soonish though.
User Story: (updated)
(In reply to Michael Buckley from comment #4)

> https://codefisher.org/catch/blog/2015/01/16/customizableui-in-thunderbird-
> and-seamonkey/

> It is working really well in Thunderbird right now, but it is not restoring
> buttons correctly in SeaMonkey.  I hope to get that fixed soonish though.

Looks interesting. Can we incorporate your code in SeaMonkey? A MPLv2 licence would be much appreciated.
Flags: needinfo?(michael.buckley)
Currently the project it is part of it MIT licensed, but you can have it as MPLv2 as well if you want.  It is all the same to me. Any official way I have to say that?

I have updated this file quite a bit since I made that post (I should go back and update it).  But the newer file is at https://github.com/codefisher/mozbutton_sdk/blob/master/templates/customizable.jsm

It is still a work in progress, I have done about 7 of the 30+ functions, though I think they are the more important/useful ones.  But for what it has, it is now a drop in replacement.

The other thing you could do to make extensions written for Australis work better, is make the panelview xul element an alias for a panel.  Everywhere you can put a panelview in SeaMonkey it should work like a panel.
Flags: needinfo?(michael.buckley)
(In reply to Michael Buckley from comment #6)
> Currently the project it is part of it MIT licensed, but you can have it as
> MPLv2 as well if you want.  It is all the same to me. Any official way I
> have to say that?

Just add the MPL header to the top of the file and then attach the file to this bug.
See https://www.mozilla.org/MPL/headers/ for the format.

> I have updated this file quite a bit since I made that post (I should go
> back and update it).  But the newer file is at
> https://github.com/codefisher/mozbutton_sdk/blob/master/templates/
> customizable.jsm

Great!

> It is still a work in progress, I have done about 7 of the 30+ functions,
> though I think they are the more important/useful ones.  But for what it
> has, it is now a drop in replacement.

> The other thing you could do to make extensions written for Australis work
> better, is make the panelview xul element an alias for a panel.  Everywhere
> you can put a panelview in SeaMonkey it should work like a panel.
I think this should be done in a separate bug.
Attached file customizable.jsm
(In reply to Michael Buckley from comment #8)
> Created attachment 8597621 [details]
> customizable.jsm

Great! Thanks!
CustomizableUI.jsm checkpoint 1
1. Reformatted to match Mozilla style guide.
2. Removed Cc, Ci, Cu shortcuts.
3. Import and use Services.jsm.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
CustomizableUI.jsm checkpoint 1.1
1. Reformatted to match Mozilla style guide.
2. Removed Cc, Ci, Cu shortcuts.
3. Import and use Services.jsm.
4. Fix spelling mistake s/Implimented/Implemented/g
Attachment #8624412 - Attachment is obsolete: true
Comment on attachment 8625072 [details] [diff] [review]
Patch v1.1 Reformat and add to build, fix spelling error

Just asking for preliminary feedback and bikeshedding. Currently most methods are stubs that throw. *This* bug is for landing the file. Implementing methods should be in follow-up bugs.
Attachment #8625072 - Flags: feedback?(neil)
Attachment #8625072 - Flags: feedback?(mnyromyr)
Attachment #8625072 - Flags: feedback?(michal-ok)
Attachment #8625072 - Flags: feedback?(iann_bugzilla)
Attachment #8625072 - Flags: feedback?(bugzilla)
I don't think I am familiar enough with the CustomizableUI code to be able to have any opinion on your proposed changed. I could only test some Fx extensions that use it if I had a build of SM with your patch applied.
(In reply to lemon_juice from comment #14)
> I don't think I am familiar enough with the CustomizableUI code to be able
> to have any opinion on your proposed changed.
That's OK. feedback can also mean take it out for a spin and let us know if it works.

> I could only test some Fx
> extensions that use it if I had a build of SM with your patch applied.

Uploaded to http://seamonkey.callek.net/contrib/
seamonkey-2.38a1.en-US.win32-20150630.zip
I've tested it with a most basic extension https://github.com/jvillalobos/Australis-Hello-World, which only puts a button with a graphic icon in the toolbar. In SM it was partially successful - the button became available in the customize window (it wasn't placed automatically on the toolbar like in Firefox - but that's fine) and I could move it to toolbars and the click action worked. The problem was the icon did not appear at all so the button was recognizable only on toolbars that had text labels enabled.
Attachment #8625072 - Flags: feedback?(michal-ok) → feedback+
Comment on attachment 8625072 [details] [diff] [review]
Patch v1.1 Reformat and add to build, fix spelling error


>+  addWidgetToArea: function(aWidgetId, aArea, aPosition) {
>+    // we are not yet handeling aPosition
Nit: handling, also should this be a TODO?

>+  registerArea: function(aName, aProperties) {
>+    // TODO: we need to do something about tracking the currentset for
>+    // registered toolbars
Nit: missing .

>+  unregisterArea: function(aName, aDestroyPlacements) {
>+    try {
>+      gAreas.delete(aName);
>+    } catch (e) {}
>+    callWithEachWindow(function(win) {
>+      var doc = win.document;
For consistency shouldn't this be document rather than doc?

>+  endBatchUpdate: function(aForceDirty) {
>+    // do we care?
Nit: Do rather than do.

>+  registerToolbarNode: function(aToolbar, aExistingChildren) {
>+    // do we need this?  It should not really be used by anyone.
Nit: Do rather than do.

>+  removeWidgetFromArea: function(aWidgetId) {
>+    callWithEachWindow(function(win) {
>+      var doc = win.document;
For consistency shouldn't this be document rather than doc?

>+function getButton(aButtonId, toolbar) {
Shouldn't this be aToolbar?

>+function restoreToToolbar(toolbox, aWidgetId) {
Shouldn't this be aToolbox?

>+function addWidgetToWindow(window, aProperties) {
Should this be aWindow?

>+function addWidgetToArea(window, aWidgetId, aArea, aPosition) {
Should this be aWindow?

>+  // we are not yet handeling aPosition
Nit: handling not handeling

>+function registerAreaForWindow(win, aName, aProperties) {
Should this be aWindow instead of win?

>+function handleWindowLoad(window) {
Should this be aWindow?

>+function callWithEachWindow(func) {
aFunction instead

>+function log(e) {
maybe aError or aMessage?

>+var windowListener = {
>+  hwl: handleWindowLoad,
>+  onOpenWindow: function(aWindow) {
>+    // Wait for the window to finish loading
Nit: missing .
Attachment #8625072 - Flags: feedback?(iann_bugzilla) → feedback+
Comment on attachment 8625072 [details] [diff] [review]
Patch v1.1 Reformat and add to build, fix spelling error

>+        button.parentNode.removeChild(button);
button.remove();

>+        var toolboxes = document.getElementsByTagName("toolbox");
>+        for (var t = 0; t < toolboxes.length; t++) {
>+          try {
>+            var toolbox = toolboxes[t];
>+            if (!toolbox.palette) {
>+              continue;
>+            }
>+            var buttons = toolbox.palette.getElementsByAttribute("id", aWidgetId);
>+            for (var i = 0; i < buttons.length; i++) {
>+              toolbox.palette.removeChild(buttons[i]);
>+            }
>+          } catch (e) {
>+            log(e);
>+          }
>+        }
(I guess this code was written to work in both SeaMonkey and Thunderbird; of course this bit will never happen because our toolboxpalettes remain in the DOM.)

>+    var old = [];
>+    for (let aProperties of gWidgets) {
>+      if (aProperties.id == aWidgetId) {
>+        old.push(aProperties);
>+      }
>+    }
>+    for (var i = 0; i < old.length; i++) {
>+      try {
>+        gWidgets.delete(old[i]);
>+      } catch (e) {}
>+    }
I'm pretty sure you can delete set elements as you enumerate them.

>+          toolbar.setAttribute("currentset", "");
"__empty"?

>+  removeListener: function(aListener) {
>+    if (aListener == this) {
>+      return;
>+    }
[?]

>+          var currentSet = toolbar.getAttribute("currentset");
>+          toolbar.setAttribute("currentset", currentSet.replace(aWidgetId, ""));
Rather than attempting to recalculate the current set yourself, just ask the toolbar.currentSet and it will recalculate it for you.

>+          toolbar.persist(toolbar, "currentset");
[This only works for built-in toolbars.]

>+    var buttonSet = toolbar.getAttribute("currentset");
...
>+        toolbar.setAttribute("currentset", buttonSet);
>+        document.persist(toolbar.id, "currentset");
Value of buttonSet doesn't actually change.

>+  if (aProperties.toolbox) {
>+    var toolboxId = aProperties.toolbox;
>+  } else {
>+    var toolboxId = "navigator-toolbox";
>+  }
>+  var toolbox = document.getElementById(toolboxId);
(aProperties.toolbox || "navigator-toolbox")

>+          toolbox.setAttribute("_addeddefaultset", buttonSet);
>+          document.persist(toolbox.id, "_addeddefaultset");
???

>+  toolbar.insertItem(aWidgetId, null, null, false);
>+  document.persist(aArea, "currentset");
You didn't actually update the current set.

>+function registerAreaForWindow(win, aName, aProperties) {
>+  let document = win.document;
>+  if (!document) {
>+    return;
>+  }
>+  var toolbar = document.getElementById(aName);
>+  if (!toolbar) {
>+    return;
>+  }
>+  if (toolbar.hasAttribute("currentset")) {
>+    var buttonSet = toolbar.getAttribute("currentset");
>+  } else {
>+    var buttonSet = toolbar.getAttribute("defaultset");
>+  }
>+  var buttons = buttonSet.split(",");
>+  for (var i in buttons) {
>+    toolbar.insertItem(buttons[i], null, null, false);
>+  }
>+  // we do this to make sure we have not changed or messed up anything by calling insertItem
>+  toolbar.setAttribute("currentset", buttonSet);
>+  document.persist(aName, "currentset");
>+}
What is this supposed to achieve?

>+function callWithEachWindow(func) {
>+  let windows = Services.wm.getEnumerator(null);
I guess this function works for any window with a customisable toolbar as long as its id is unique?

>+  while (windows.hasMoreElements()) {
>+    let domWindow = windows.getNext().QueryInterface(Components.interfaces.nsIDOMWindow);
Nit: You don't actually need to QI for nsIDOMWindow.

>+    domWindow.addEventListener("load", function onLoad() {
>+      domWindow.removeEventListener("load", onLoad, false);
>+      self.hwl(domWindow);
>+    }, false);
Ideally should just add handleWindowLoad as the event listener (and make handleWindowLoad remove itself), but at least just call handleWindowLoad directly.
Attachment #8625072 - Flags: feedback?(neil)
This bug blocks issue #158 "Add SeaMonkey support" of the passff plugin. See: https://github.com/jvenant/passff/issues/158

I don't think Phillip is working on this anymore.

Flags: needinfo?(frgrahl)

No kidding, Ratty hasn't been heard from for years now.

Assignee: philip.chee → nobody
Status: ASSIGNED → NEW
Comment on attachment 8625072 [details] [diff] [review]
Patch v1.1 Reformat and add to build, fix spelling error

No Ratty left the project ( or at least we must assume it). I looked at it CustomizableUI a while beack but when we implement it current NoScript 5.1.x will break. Seems it is used there to check for Firefox. Maybe for 2.57.
Flags: needinfo?(frgrahl)
Attachment #8625072 - Flags: feedback?(mnyromyr)
Attachment #8625072 - Flags: feedback?(bugzilla)
You need to log in before you can comment on or make changes to this bug.