Closed Bug 636104 Opened 13 years ago Closed 10 years ago

Redirect Lightning Preferences to the Suite Preferences window.

Categories

(Calendar :: Lightning: SeaMonkey Integration, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Original patch in Bug 518736 has r=IanN sr=Neil. Attaching this patch for pro-forma reviews.
Attachment #514443 - Flags: superreview?(neil)
Attachment #514443 - Flags: review?(iann_bugzilla)
Attachment #514443 - Flags: approval-seamonkey2.1b3?
Attachment #514443 - Flags: feedback?(philipp)
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.
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 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.
Attachment #514443 - Flags: superreview?(neil)
Attachment #514443 - Flags: review?(iann_bugzilla)
Attachment #514443 - Flags: feedback?(philipp)
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.
Attachment #514443 - Flags: approval-seamonkey2.1b3? → approval-seamonkey2.1b3-
Summary: Stub messenger preferences.xul for Lightning [SM2.1] → Redirect Lightning Preferences to the Suite Preferences 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.
Attachment #514443 - Attachment is obsolete: true
Attachment #8358812 - Flags: feedback?(neil)
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.
Attachment #8358812 - Flags: feedback?(neil)
Moving to Lightning component.
Component: Preferences → Lightning: SeaMonkey Integration
Flags: approval-seamonkey2.1b3-
Product: SeaMonkey → Calendar
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".
Attachment #8358812 - Attachment is obsolete: true
Attachment #8361151 - Flags: review?(philipp)
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.
Attachment #8361151 - Flags: review?(philipp) → review+
Pushed to comm-central:
remote:   https://hg.mozilla.org/comm-central/rev/576245902b22
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.1
Blocks: 1267666
You need to log in before you can comment on or make changes to this bug.