288313 - Hook up connection/proxy prefs to Sunbird

Status: VERIFIED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Simon Paquet [:sipaq]]

Comment 1

[Simon Paquet [:sipaq]]

Attachment: Patch v1

This is the patch, it needs four other files (preferences.properties,
connection.xul, connection.js and xonnection.dtd) to be fully functional. I
will attach these files shortly after this patch.

Comment 2

[Simon Paquet [:sipaq]]

Attachment: connection.xul

This needs to go into mozilla/calendar/resources/content/pref

Comment 3

[Simon Paquet [:sipaq]]

Attachment: connection.js

This needs to go into mozilla/calendar/resources/content/pref

Comment 4

[Simon Paquet [:sipaq]]

Attachment: preferences.properties

This needs to go into mozilla/calendar/resources/locale/en-US/

Comment 5

[Simon Paquet [:sipaq]]

Attachment: connection.dtd

This needs to go into mozilla/calendar/resources/locale/en-US/

Comment 6

[Mostafa Hosseini]

r+ for all the files being added. The patch is a bit out of date because jar.mn
has changed since but the change seems to be easy to fix. sipaq: If you won't
mind updating the patch, please do so and I will r+ it right after.

Comment 7

[Simon Paquet [:sipaq]]

Mostafah, thanks for the review. I will update the patch on Friday, when I'm at
my developing machine.

Comment 8

[Simon Paquet [:sipaq]]

Attachment: Patch v2

Updated patch

Comment 9

[Mostafa Hosseini]

Comment on attachment 180076 [details] [diff] [review]
Patch v2

r=mostafah
I agree that this should go into the tree until the proxy settings move to
toolkit so I r+ it. However this is going to benefit only sunbird and be just a
burden to the extension or lightning. In case there is a voice among the
developers that doesn't agree I would prefer to have a second review before
checking in.mvl would you mind doing the second review please?

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

I wonder how much work it would be to not copy those files into calendar, but
into toolkit.

Comment 11

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 179065 [details]
connection.xul


>    <preferences>
>      <preference id="network.proxy.type"         name="network.proxy.type"         type="int" onchange="gConnectionsDialog.proxyTypeChanged();"/>
Is it a good iea to mix old style prefwindows with newstyle? Should we update
the sunbird prefwindow first?
Also, how much work would it to just move the files into toolkit? If it is just
asmuch work as copying into sunbird, why not do the right thing?

Comment 12

[Simon Paquet [:sipaq]]

(In reply to comment #11)
> Is it a good idea to mix old style prefwindows with newstyle? Should we update
> the sunbird prefwindow first?

You're right, it isn't a good idea. From my perspective it would be better if we
could move to the newstyle prefwindows alltogether, but I'm not up to that task.

What I can do though, is trying to hook up the old proxy-pref-window to Sunbird. 

So I'm gonna need a decision from you (mvl) and Mostafah. I have a long weekend
ahead and can put some time into this, but I would prefer working on something
that you actually want.

> Also, how much work would it to just move the files into toolkit? If it is 
> just as much work as copying into sunbird, why not do the right thing?

If we decide to use the newstyle prefwindows, I would suggest moving the proxy
prefs to Toolkit. It is just as much work as copying into Sunbird.

Comment 13

[Simon Paquet [:sipaq]]

Attachment: Patch v3

New patch using the oldstyle prefwindow files connectionPrefs.xul and
connectionsPrefs.js, which I will add separately. Carrying over review+ by
Mostafah

Comment 14

[Simon Paquet [:sipaq]]

Attachment: connectionPrefs.js

This will have to go into mozilla/calendar/sunbird/base/content/pref/

Comment 15

[Simon Paquet [:sipaq]]

Attachment: connectionPrefs.xul

This will have to go into mozilla/calendar/sunbird/base/content/pref/

Comment 16

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 182698 [details] [diff] [review]
Patch v3

r=mvl, but instead of adding the locale files in the shared resources/jar.mn,
they should be added in the sunbird jar.mn

Comment 17

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 182698 [details] [diff] [review]
Patch v3

actually, this is wrong. It  will make the button for the connection prefs show
up in every app, while it should only show up for sunbird.
I think this can be solved by using a overlay that's only build by sunbird.

Comment 18

[Simon Paquet [:sipaq]]

(In reply to comment #16)
> (From update of attachment 182698 [details] [diff] [review] [edit])
> r=mvl, but instead of adding the locale files in the shared resources/jar.mn,
> they should be added in the sunbird jar.mn

Excuse me, but how do I do that? As far as I know, I can only include files in
or below the directory of the jar.mn file into the jar file. 

Should I move the locale files into calendar/sunbird/base/locale?
IIRC in a IRC discussion with you, shaver and me we decided that it was ok to
leave these files in resources/locale. The only thing possible from my point of
view would be to #ifdef MOZ_SUNBIRD these entries in resources/jar.mn

Comment 19

[Simon Paquet [:sipaq]]

(In reply to comment #17)
> (From update of attachment 182698 [details] [diff] [review] [edit])
> actually, this is wrong. It will make the button for the connection prefs show
> up in every app, while it should only show up for sunbird.
> I think this can be solved by using a overlay that's only build by sunbird.

Should we really use an overlay just for this? Wouldn't an #ifdef MOZ_SUNBIRD be
enough?

Comment 20

[Michiel van Leeuwen (email: mvl+moz@)]

(In reply to comment #18)
> Should I move the locale files into calendar/sunbird/base/locale?

Yeah, i think that would work. I don't see why not. (And i can't remember the
discussion. What were the reasons to put them in the shared location?)


(In reply to comment #19)
> Should we really use an overlay just for this? Wouldn't an #ifdef MOZ_SUNBIRD be
> enough?

That would work, but i'm generally not to happy about ifdefs. But since this is
supposed to be temporary (file a followup bug!), it's ok with me.

Comment 21

[Simon Paquet [:sipaq]]

Attachment: Patch v4

New patch with review comments fixed.

Comment 22

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 182967 [details] [diff] [review]
Patch v4

jar.mn:
>+#ifdef MOZ_SUNBIRD
>+endif

You missed an # there. And in the other places where you used #ifdef.

Comment 23

[Simon Paquet [:sipaq]]

Attachment: Patch v5

Comment 24

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 183528 [details] [diff] [review]
Patch v5

r=mvl, with the exception of thos dos line endings that sneaked in.

Comment 25

[Michiel van Leeuwen (email: mvl+moz@)]

patch and files checked in.

Comment 26

[David G King]

Getting a compile error on Win32/MinGW/cygwin

error: file '/cygdrive/e/mozilla/source/mozilla/calendar/resources/locale/en-US/
connectionPrefs.dtd' doesn't exist at /cygdrive/e/mozilla/source/mozilla/config/
make-jars.pl line 428.
make[4]: *** [libs] Error 2

Comment 27

[Simon Paquet [:sipaq]]

mvl, somehow the patch you applied didn't create connectionPrefs.dtd and
prefutilities.properties. Both files haven't been checked in, which obviously
breaks the build.

Comment 28

[Michiel van Leeuwen (email: mvl+moz@)]

added missing files.

Comment 29

[Simon Paquet [:sipaq]]

*** Bug 230698 has been marked as a duplicate of this bug. ***

Comment 30

[Simon Paquet [:sipaq]]

*** Bug 281603 has been marked as a duplicate of this bug. ***

Comment 31

[Joey Minta]

*** Bug 310224 has been marked as a duplicate of this bug. ***