285570 - add entities for changing window style (height,width) of preferences windows

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P2)

Description

[Reuven Gonen]

Comment 1

[Asaf Romano (gone)]

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

Comment 2

[Reuven Gonen]

Attachment: connection settings screenshot

currently the style of some child windows under preferences are specified in
width inside the xul. it should be possible to change the width by the locale.

i'm attaching a screenshot of connection settings window with the he-IL locale.

Comment 3

[Reuven Gonen]

Attachment: moving the windows width to dtd files

I've set the style settings inside the locale files. this dosen't address the
size diffrence between systems (mac,win,unix).

Comment 4

[Reuven Gonen]

Attachment: fixed patch

I'm removing a typo from permissions.dtd

Comment 5

[Asaf Romano (gone)]

Comment on attachment 177292 [details] [diff] [review]
fixed patch

> style="&Window.size;"

* The style attribute /can/ contain any inline style bit, so "Window.size" is
not the right title; "window.style" would be better (also, please don't
capitalize entities).

> <!ENTITY window.title                 "Exceptions">
>+<!ENTITY  Window.size     "width: 36em;">

* indent is wrong, it should be:

> <!ENTITY window.title                 "Exceptions">
>+<!ENTITY window.style                 "width: 36em;">

(That is only one instace, please fix the rest as well)

* You haven't patched downloadactions.xul.

Also, I'm not a browser/ peer, once you've a ready patch, please request r=
from mconnor ;).

Comment 6

[Asaf Romano (gone)]

-> Reuven

Comment 7

[Benjamin Smedberg]

This should not be done with CSS properties, it should be done with entities for
the width/height attribute.

Comment 8

[Reuven Gonen]

(In reply to comment #7)
> This should not be done with CSS properties, it should be done with entities for
> the width/height attribute.

do you mean we shall remove the styles? in this case we lose the !important flag
that may be required in some cases.

second, do we want to set a diffrent window width to diffrent OS? this may solve
bug 285574 as well.

Comment 9

[Asaf Romano (gone)]

> second, do we want to set a diffrent window width to diffrent OS? this may solve
> bug 285574 as well.

we will, for mac. but don't mess with this in this one.

Comment 10

[Nickolay_Ponomarev]

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

Comment 11

[Reuven Gonen]

Attachment: alternate fix, moving only the width,height size to dtd

in this patch i moved only the size settings to the dtd. this gives less chance
for translating the "!important" tag, or missing a ; by mistake. it also leaves
more control in the xul files.

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 180284 [details] [diff] [review]
alternate fix, moving only the width,height size to dtd

for all of the .dtd files:

fix the spacing/alignment for the entity name and use spaces instead of tabs to
fix the alignment of the values.

With that fixed, r=me

Comment 13

[Reuven Gonen]

Attachment: using spaces instead of tabs for entity values

this patch fixes the alignment of the entity values in the dtd files.

Comment 14

[(not currently active) Ted Mielczarek]

Comment on attachment 181057 [details] [diff] [review]
using spaces instead of tabs for entity values

Surely you didn't give yourself r+

Comment 15

[Benjamin Smedberg]

Comment on attachment 181057 [details] [diff] [review]
using spaces instead of tabs for entity values

Ted, he did not "give himself r+", he marked mconnor's review from the previous
patch.

Comment 16

[Reuven Gonen]

the alignment in fonts.dtd needs yet to be corrected:
https://bugzilla.mozilla.org/attachment.cgi?id=181057&action=diff#mozilla/browser/locales/en-US/chrome/browser/preferences/fonts.dtd_sec1

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 181057 [details] [diff] [review]
using spaces instead of tabs for entity values

a=asa

Comment 18

[Asaf Romano (gone)]

Checking in components/preferences/changeaction.xul;
/cvsroot/mozilla/browser/components/preferences/changeaction.xul,v  <-- 
changeaction.xul
new revision: 1.4; previous revision: 1.3
done
Checking in components/preferences/colors.xul;
/cvsroot/mozilla/browser/components/preferences/colors.xul,v  <--  colors.xul
new revision: 1.3; previous revision: 1.2
done
Checking in components/preferences/connection.xul;
/cvsroot/mozilla/browser/components/preferences/connection.xul,v  <-- 
connection.xul
new revision: 1.4; previous revision: 1.3
done
Checking in components/preferences/cookies.xul;
/cvsroot/mozilla/browser/components/preferences/cookies.xul,v  <--  cookies.xul
new revision: 1.4; previous revision: 1.3
done
Checking in components/preferences/downloadactions.xul;
/cvsroot/mozilla/browser/components/preferences/downloadactions.xul,v  <-- 
downloadactions.xul
new revision: 1.6; previous revision: 1.5
done
Checking in components/preferences/fonts.xul;
/cvsroot/mozilla/browser/components/preferences/fonts.xul,v  <--  fonts.xul
new revision: 1.6; previous revision: 1.5
done
Checking in components/preferences/languages.xul;
/cvsroot/mozilla/browser/components/preferences/languages.xul,v  <--  languages.xul
new revision: 1.3; previous revision: 1.2
done
Checking in components/preferences/permissions.xul;
/cvsroot/mozilla/browser/components/preferences/permissions.xul,v  <-- 
permissions.xul
new revision: 1.4; previous revision: 1.3
done
Checking in components/preferences/sanitize.xul;
/cvsroot/mozilla/browser/components/preferences/sanitize.xul,v  <--  sanitize.xul
new revision: 1.4; previous revision: 1.3
done
Checking in locales/en-US/chrome/browser/preferences/changeaction.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/changeaction.dtd,v
 <--  changeaction.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/colors.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/colors.dtd,v 
<--  colors.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/connection.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/connection.dtd,v
 <--  connection.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/cookies.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/cookies.dtd,v
 <--  cookies.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/downloadactions.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/downloadactions.dtd,v
 <--  downloadactions.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/fonts.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/fonts.dtd,v 
<--  fonts.dtd
new revision: 1.4; previous revision: 1.3
done
Checking in locales/en-US/chrome/browser/preferences/languages.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/languages.dtd,v
 <--  languages.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/permissions.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/permissions.dtd,v
 <--  permissions.dtd
new revision: 1.3; previous revision: 1.2
done
Checking in locales/en-US/chrome/browser/preferences/sanitize.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/sanitize.dtd,v
 <--  sanitize.dtd
new revision: 1.4; previous revision: 1.3
done

Comment 19

[Reuven Gonen]

Attachment: patch for ocsp.xul

seems like i missed ocsp.xul...

Comment 20

[Asaf Romano (gone)]

Checking in locales/en-US/chrome/mozapps/preferences/ocsp.dtd;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/preferences/ocsp.dtd,v 
<--  ocsp.dtd
new revision: 1.2; previous revision: 1.1
done
Checking in mozapps/preferences/ocsp.xul;
/cvsroot/mozilla/toolkit/mozapps/preferences/ocsp.xul,v  <--  ocsp.xul
new revision: 1.3; previous revision: 1.2
done

Comment 21

[Reuven Gonen]

Attachment: parallel patch for Thunderbird

this patch is for fixing Thunderbird preferences windows

Comment 22

[Asaf Romano (gone)]

please attach it in a new bug.

Comment 23

[Reuven Gonen]

i opened a new bug for thunderbird:
Bug 298796

Comment 24

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change