Closed Bug 1302830 Opened 8 years ago Closed 8 years ago

Remove preprocessing from mail/base/aboutDialog.js

Categories

(Thunderbird :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 51.0

People

(Reporter: aceman, Assigned: aceman)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1302005 comment 3+++

Port removal of preprocessing from mail/base/aboutDialog.js from browser/base/content/aboutDialog.js .
Attached patch patchSplinter Review
The patch removes preprocessing from the aboutDialog.js file by using AppConstants instead of defines, similarly to the Firefox file aboutDialog.js. It also moves the updater code to a separate file aboutDialog-appUpdater.js as Firefox has it. I am not sure what the benefit of that is (the file is not included from multiple files). Maybe just that you do not have imports in the middle of a file and the whole code is a single object in a separate file. But I just move the code that was in the TB version of aboutDialog.js without any changes (no syncing with FF there).
Attachment #8791548 - Flags: review?(jorgk)
Comment on attachment 8791548 [details] [diff] [review]
patch

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

What's the overall benefit of this other than adapting a little more to the FF ways?

::: mail/base/jar.mn
@@ +51,5 @@
>      content/messenger/specialTabs.js                (content/specialTabs.js)
>      content/messenger/specialTabs.xul               (content/specialTabs.xul)
>      content/messenger/subscribe.xul                 (content/subscribe.xul)
>      content/messenger/subscribe.js                  (content/subscribe.js)
> +    content/messenger/aboutDialog-appUpdater.js     (content/aboutDialog-appUpdater.js)

Just enlighten me what this is all about. What is MOZ_UPDATE? and what does that JS file do?

@@ +56,2 @@
>  *   content/messenger/aboutDialog.xul               (content/aboutDialog.xul)
> +    content/messenger/aboutDialog.js                (content/aboutDialog.js)

And what does the * do? Request preprocessing?
(In reply to Jorg K (GMT+2, PTO during summer) from comment #2)
> What's the overall benefit of this other than adapting a little more to the
> FF ways?

Removing preprocessing is a bug benefit for devs (at least on Linux). I see no benefit of splitting code into the new file, just adapting to FF.

> ::: mail/base/jar.mn
> @@ +51,5 @@
> >      content/messenger/specialTabs.js                (content/specialTabs.js)
> >      content/messenger/specialTabs.xul               (content/specialTabs.xul)
> >      content/messenger/subscribe.xul                 (content/subscribe.xul)
> >      content/messenger/subscribe.js                  (content/subscribe.js)
> > +    content/messenger/aboutDialog-appUpdater.js     (content/aboutDialog-appUpdater.js)
> 
> Just enlighten me what this is all about. What is MOZ_UPDATE? and what does
> that JS file do?

It is the updater that downloads a new TB version. You can compile with MOZ_UPDATER disabled, then you do not have that functionality. That define allows to disable its code.

> 
> @@ +56,2 @@
> >  *   content/messenger/aboutDialog.xul               (content/aboutDialog.xul)
> > +    content/messenger/aboutDialog.js                (content/aboutDialog.js)
> 
> And what does the * do? Request preprocessing?

Yes. Then you need to build whole TB when you change the .js file. If a file does not need preprocessing, you can edit it and have the changes picked up by the binary without rebuilding (sometimes even without restarting TB). At least on Linux. Not sure if this also works on Windows.
Comment on attachment 8791548 [details] [diff] [review]
patch

OK ;-)
Attachment #8791548 - Flags: review?(jorgk) → review+
https://hg.mozilla.org/comm-central/rev/b92e9d797f12c47fb2804c5a480427e4709f74cd
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 51.0
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: