1508119 - Calendar/Lightning still uses XUL overlays internally

Status: RESOLVED FIXED

(Calendar :: General, task)

Description

[Worcester12345]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

From https://bugzilla.mozilla.org/show_bug.cgi?id=1493008

Comment 3:
https://bugzilla.mozilla.org/show_bug.cgi?id=1493008#c3



Actual results:

This needs to happen to solve one of the problems here:

Bug 1493008
Please remove all Addon/Extension code from Lightning, and integrate full calendar functionality directly into Thunderbird



Expected results:

Remove XUL overlays from Calendar/Lightning

Comment 1

[Worcester12345]

Is it time to start working on these?

Comment 2

[Paul Morris [:pmorris]]

Work on removing overlay use by calendar should happen in two phases:

• Removing the overlays that calendar is using internally.
• Integrating calendar into Thunderbird so we no longer use overlays to add calendar to Thunderbird's UI.

I'm going to use this bug for the first phase, which can be an early step on the path to integrating calendar into Thunderbird. We can open a separate bug for the second phase if needed when the time comes. (That will be either bug 1508119 or a sub-bug.)

Comment 3

[Paul Morris [:pmorris]]

Attachment: part1-calendar-common-sets-xul-0.patch

1 of 2: de-overlay/inline the calendar-common-sets.xul file.

Comment 4

[Paul Morris [:pmorris]]

Attachment: part2-calendar-common-sets-js-0.patch

2 of 2: Since there won't be a calendar-common-sets.xul file, rename the calendar-common-sets.js file to make more sense. (And also move a misplaced minimonth function that wasn't supposed to be in that file, while we're at it.)

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9103042 [details] [diff] [review]
part1-calendar-common-sets-xul-0.patch

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

Since this is a cut/paste job I haven't looked too closely. I hope you've run the tests.

::: calendar/lightning/content/messenger-overlay-sidebar.xul
@@ +196,5 @@
>        <key id="calendar-new-todo-key" key="&lightning.keys.todo.new;" modifiers="accel" command="calendar_new_todo_command"/>
> +
> +// For linux tab switching reserves alt+number, where on windows that's ctrl.
> +// Use the available modifiers for each platform.
> +// Can't use the OPTION key on OSX, so we will use SHIFT+OPTION on the Mac.

Let's make this a real XML comment while we're at it.

@@ +301,5 @@
>                    command="calendar_edit_calendar_command"/>
>        </menupopup>
> +
> +      <!-- CALENDAR ITEM CONTEXT MENU -->
> +      <menupopup id="calendar-item-context-menu" onpopupshowing="return setupContextItemType(event, currentView().getSelectedItems({}));">

Split this line up.

Comment 6

[Paul Morris [:pmorris]]

Thanks for review. Setting the 'leave-open' flag, plan to use this bug for all these de-overlay patches.

Comment 7

[Paul Morris [:pmorris]]

Attachment: part1-calendar-common-sets-xul-1.patch

Review comments addressed. Tests passed locally. Try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d78ffee134bccb55ea0e269a96d12d72ab3318c5

Comment 8

[Paul Morris [:pmorris]]

Try run looks OK. checkin-needed for part1 and part2.

Comment 9

[Paul Morris [:pmorris]]

Attachment: part3-today-pane-xul-0.patch

part3 - Inlines the today-pane.xul file.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/454c16bf090d
Inline the calendar-common-sets.xul overlay file. r=darktrojan
https://hg.mozilla.org/comm-central/rev/e0e759a38367
Convert calendar-common-sets.js to calendar-command-controller.js. r=darktrojan

Comment 11

[Paul Morris [:pmorris]]

Here is a categorization of all the overlay files in Calendar, to give an overview of the scope of this bug and to help with the various integration questions.

FILES TO INLINE (the work of this bug):

caldav-lightning-calendar-creation.xul (inlined in bug 1586371)
caldav-lightning-calendar-properties.xul (inlined in bug 1568660)
calendar-common-sets.xul
calendar-task-view.xul
calendar-unifinder.xul
calendar-views.xul
lightning-calendar-creation.xul (inlined in bug 1586371)
lightning-calendar-properties.xul (inlined in bug 1568660)
lightning-menus.xul
lightning-toolbar.xul
today-pane.xul

FILES THAT OVERLAY MORE THAN ONE CALENDAR FILE (also this bug):

lightning-item-toolbar.xul (two calendar files)

FILES THAT OVERLAY TB FILES:

lightning-item-panel.xul
lightning-migration.xul (Still needed? It's from September 2008.) (removed in bug 1594581)
messenger-overlay-accountCentral.xul
messenger-overlay-messageWindow.xul
messenger-overlay-sidebar.xul
imip-bar-overlay.xul (a TB file and a calendar file)
messenger-overlay-preferences.xul (a TB file and a suite file)

FILES THAT ARE SUITE RELATED (e.g. THAT OVERLAY SUITE FILES):

suite-overlay-preferences.xul
suite-overlay-addons.xul
suite-overlay-sidebar.xul (overlays messenger-overlay-sidebar.xul)

Comment 12

[Geoff Lankow (:darktrojan)]

Comment on attachment 9103392 [details] [diff] [review]
part3-today-pane-xul-0.patch

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

::: calendar/lightning/content/messenger-overlay-sidebar.xul
@@ +86,5 @@
>    <script src="chrome://calendar/content/calendar-command-controller.js"/>
>  
> +  <!-- NEEDED FOR TODAY PANE -->
> +  <script src="chrome://global/content/globalOverlay.js"/>
> +  <script src="chrome://global/content/editMenuOverlay.js"/>

messenger.xul already has these two.

@@ +1128,5 @@
>      </splitter>
> +    <calendar-modevbox id="today-pane-panel"
> +                      mode="mail,calendar,task,chat"
> +                      modewidths="200,200,200,200"
> +                      modesplitterstates="open,open,open,open"

Indentation needs fixing all the way down this chunk.

Comment 13

[Paul Morris [:pmorris]]

Attachment: part3-today-pane-xul-1.patch

Thanks for the review and for catching the indentation thing. My editor is not as smart as it thinks it is when re-indenting blocks of XML. Would that we had auto-formatters for that.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=184f10041c4bd09ecab2a1d6bd700d5e43f77295

Comment 14

[Paul Morris [:pmorris]]

checkin-needed for part3, assuming no issues in the try run.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/58c9543097ad
Inline the today-pane.xul overlay file. r=darktrojan

Comment 16

[Paul Morris [:pmorris]]

Attachment: part4-calendar-task-view-xul-0.patch

part4 - Inline calendar-task-view.xul

Comment 17

[Paul Morris [:pmorris]]

Attachment: part5-calendar-unifinder-xul-0.patch

part5 - Inline calendar-unifinder.xul

Try run for parts 4 and 5:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e04366d957d47d623a3084d1d06298caed452ac6

Comment 18

[Paul Morris [:pmorris]]

part4 and part5 ready for checking in.

Comment 19

[Jorg K (CEST = GMT+2)]

Thanks. Leaving this for landing in ~6 hours. I need to spread out the patches I have.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9088ce19d9d4
Inline the calendar-task-view.xul overlay file. r=darktrojan
https://hg.mozilla.org/comm-central/rev/7edaf5c980d3
Inline the calendar-unifinder.xul overlay file. r=darktrojan

Comment 21

[Paul Morris [:pmorris]]

Attachment: part6-calendar-views-xul-0.patch

part6 - calendar-views.xul

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/029af9f181ab
Inline the calendar-views.xul overlay file. r=darktrojan

Comment 23

[Paul Morris [:pmorris]]

Attachment: part7-lightning-toolbar-xul-0.patch

part7 - calendar-toolbar.xul - Even though messenger.xul has chrome://branding/locale/brand.dtd I got a XML parsing error if it wasn't added to the messenger-overlay-sidebar.xul file here. The &brandShortName; in this line was causing the error:

<!ENTITY lightning.toolbar.appmenuButton1.tooltip "Display the &brandShortName; Menu">

Comment 24

[Paul Morris [:pmorris]]

Attachment: part8-lightning-menus-xul-0.patch

part8 - lightning-menus.xul

Try run for parts 7 and 8:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b593cd02fdf6467c3d7f59e826af3b0eafc22362

Comment 25

[Geoff Lankow (:darktrojan)]

Comment on attachment 9106414 [details] [diff] [review]
part8-lightning-menus-xul-0.patch

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

I'm assuming this is a straight copy/paste job and not reading it all.

Comment 26

[Jorg K (CEST = GMT+2)]

Just landing one, keeping the other for later.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/aa5d53e8d88c
Inline the lightning-toolbar.xul overlay file. r=darktrojan

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/20c8f5c7d6e5
Inline the lightning-menus.xul overlay file. r=darktrojan

Comment 29

[Paul Morris [:pmorris]]

Attachment: part9-lightning-item-toolbar-0.patch

Part 9 - This one is used in two places so convert it to being included by the pre-processor (rather than just inlining it). Try run:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bba5bfb18342cd5f6e5068537e3cfec4ec979a36

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f5895f13fccb
De-overlay the lightning-item-toolbar.xul file. r=darktrojan

Comment 31

[Paul Morris [:pmorris]]

Attachment: part10-remove-overlay-directives-0.patch

part 10 - There were a couple of overlay directives left over from the two XUL files that were deleted (inlined) in bug 1586371.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e4f02a3ecd47
Remove left-over overlay directives from bug 1586371. r=darktrojan

Comment 33

[Paul Morris [:pmorris]]

All the overlays internal to calendar are now inlined or included by pre-processing, so it's time to close this bug.

I'll open a separate bug for dealing with the suite (seamonkey) related overlays:

• suite-overlay-preferences.xul
• suite-overlay-addons.xul
• suite-overlay-sidebar.xul (overlays messenger-overlay-sidebar.xul)

That leaves the files that overlay TB files that will be handled as part of the integration effort.

• lightning-item-panel.xul
• messenger-overlay-accountCentral.xul
• messenger-overlay-messageWindow.xul
• messenger-overlay-sidebar.xul
• imip-bar-overlay.xul (a TB file and a calendar file)
• messenger-overlay-preferences.xul (a TB file and a suite file)