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)
Calendar
Lightning: SeaMonkey Integration
Tracking
(Not tracked)
RESOLVED
FIXED
3.1
People
(Reporter: philip.chee, Assigned: philip.chee)
References
Details
Attachments
(1 file, 2 obsolete files)
5.94 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
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.
![]() |
Assignee | |
Comment 1•13 years ago
|
||
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?
Updated•13 years ago
|
Attachment #514443 -
Flags: feedback?(philipp)
![]() |
||
Comment 2•13 years ago
|
||
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.
![]() |
Assignee | |
Comment 3•13 years ago
|
||
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?
![]() |
Assignee | |
Comment 4•13 years ago
|
||
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 5•13 years ago
|
||
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-
![]() |
Assignee | |
Updated•10 years ago
|
Summary: Stub messenger preferences.xul for Lightning [SM2.1] → Redirect Lightning Preferences to the Suite Preferences window.
![]() |
Assignee | |
Comment 6•10 years ago
|
||
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 7•10 years ago
|
||
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)
![]() |
Assignee | |
Comment 8•10 years ago
|
||
Moving to Lightning component.
Component: Preferences → Lightning: SeaMonkey Integration
Flags: approval-seamonkey2.1b3-
Product: SeaMonkey → Calendar
![]() |
Assignee | |
Comment 9•10 years ago
|
||
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 10•10 years ago
|
||
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+
![]() |
Assignee | |
Comment 11•10 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•