636104 - Redirect Lightning Preferences to the Suite Preferences window.

Status: RESOLVED FIXED

(Calendar :: Lightning: SeaMonkey Integration, defect)

Description

[Philip Chee]

From Bug 518736 Comment 48:
> I can make a trunk patch to do this (I have the code running locally) but I'm
> holding back until the UI (and API) stabilizes since I don't want to do useless
> work.

Forward port the patch from Bug 518736 to SeaMonkey 2.1.

Comment 1

[Philip Chee]

Attachment: Stub messenger preferences.xul for Lightning [SM2.1] from Bug 518736.

Original patch in Bug 518736 has r=IanN sr=Neil. Attaching this patch for pro-forma reviews.

Comment 2

[Robert Kaiser]

I will not approve this because I'm still opposed to fixing a Lightning bug in SeaMonkey permanently, a fix really belongs in Lightning IMHO and that's why I vetoed this going onto trunk in the first place. But as I'm fading out of the project, I'm not vetoing at this stage but only stating my opinion.

Comment 3

[Philip Chee]

How about attachment 404243 [details] [diff] [review] from Bug 518736 ?
A one line patch using Javascript in the <optionsURL>:
> -    <em:optionsURL>chrome://messenger/content/preferences/preferences.xul</em:optionsURL>
> +    <em:optionsURL>javascript:var url=Application.id=='{92650c4d-4b8e-4d2a-b7eb-24ecf4f6b63a}'?'chrome://communicator/content/pref/preferences.xul':'chrome://messenger/content/preferences/preferences.xul';opener.openDialog(url,'','chrome,titlebar,toolbar,centerscreen,modal,resizable','paneLightning');window.close();</em:optionsURL>

It was previously rejected as too hacky. Perhaps the Lightning devs might reconsider?

Comment 4

[Philip Chee]

Comment on attachment 514443 [details] [diff] [review]
Stub messenger preferences.xul for Lightning [SM2.1] from Bug 518736.

I guess this version of the bug is WONTFIX.

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 514443 [details] [diff] [review]
Stub messenger preferences.xul for Lightning [SM2.1] from Bug 518736.

If and only if you get review+ will I consider the hackery-stub in our code for lightning.

Comment 6

[Philip Chee]

Attachment: Patch v1.0 Redirect Options button to the SeaMonkey Preference Window.

New approach: Redirect Lightning Preferences to the Suite Preferences window.

> +        var item;
> +        try {
> +          item = gListView.getListItemForID(this.guid);
Get the richlistitem that corresponds to Lightning.

> +        } catch (ex) {
> +        };
> +
> +        if (!item || !item.showPreferences) {
> +          Services.console
> +                  .logStringMessage("Lighting: Unknown showPreferences API");
> +          return;
> +        }
If something changed incompatibly we bail early so we are no worse off than without this patch.

> +% overlay about:addons chrome://lightning/content/suite-overlay-addons.xul ...
> +% overlay chrome://mozapps/content/extensions/extensions.xul chrome://lightning/content/suite-overlay-addons.xul ...
Overlay both because it's possible for extensions to call the addons manager with either URL.

Comment 7

[[e-mail]]

Comment on attachment 8358812 [details] [diff] [review]
Patch v1.0 Redirect Options button to the SeaMonkey Preference Window.

>+        try {
>+          item = gListView.getListItemForID(this.guid);
Don't need this, it can't throw.

>+        } catch (ex) {
>+        };
Spurious semicolon after block.

>+        if (!item || !item.showPreferences) {
>+          Services.console
>+                  .logStringMessage("Lighting: Unknown showPreferences API");
Item might legitimately not exist (wrong view selected).
showPreferences might not exist, in which case writing to it can hardly make things worse.

>+        if (win)
>+          win.focus();
[Perhaps use syncTreeWithPane to switch to the Lightning pane? See bug 798147.]

>+    window.addEventListener("load", lightningPrefs.init.bind(lightningPrefs), false);
If you use handleEvent instead then you can just add the object itself as the listener.
Listening to the load event is incorrect, you need to watch the ViewChanged event instead.

Comment 8

[Philip Chee]

Moving to Lightning component.

Comment 9

[Philip Chee]

Attachment: Patch v1.1 Redirect Options button to the SeaMonkey Preference Window.

Changes since the last patch:

1. Remove try/catch block.
2. override showPreferences method unconditionally.
2a. "showPreferences might not exist, in which case writing to it can hardly
    make things worse."
3. Use syncTreeWithPane to switch to the Lightning pane if the Preferences
   window is already open.
4. Use handleEvent instead and add the object itself as the listener.
5. Listen to the "ViewChanged" event instead of "load".

Comment 10

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8361151 [details] [diff] [review]
Patch v1.1 Redirect Options button to the SeaMonkey Preference Window.

Review of attachment 8361151 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=philipp. Only a minor nit:

::: calendar/lightning/content/suite-overlay-addons.xul
@@ +25,5 @@
> +          var pane = doc.getElementById("paneLightning");
> +          doc.documentElement.syncTreeWithPane(pane, true);
> +        }
> +        else
> +          openDialog("chrome://communicator/content/pref/preferences.xul",

brackets in one line with the else and also brackets for single-line else blocks.

Comment 11

[Philip Chee]

Pushed to comm-central:
remote:   https://hg.mozilla.org/comm-central/rev/576245902b22