183328 - Remove "other resolution" from DPI for OS/2

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Mike Kaply [:mkaply]]

On OS/2, "other resolution" shouldn't be available. You can't really customize
the DPI like you can on other operating systems.

This patch allows us to do that.

To accomplish this, I did some renaming of the otherResolution menuitem because
it was misnamed and I added IDs for the other and the separator.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Patch

Patch to fix this.

Rename otherResolution to secondaryResolution (that's what it is)

add id for separator and id of otherResolution to other resolution item.

Comment 2

[Mike Kaply [:mkaply]]

Comment on attachment 108103 [details] [diff] [review]
Patch

Gerv, are you around to take a look at this?

Comment 3

[Gervase Markham [:gerv]]

I'm not convinced by the renaming. It creates an inconsistency with e.g.
label="&resolution.other;", and if you changed that as well, it would make life
unnecessarily more complicated for localisers, and further bloat the patch. The
name "otherResolution" seems at least reasonable, given that there's two of
them. I certainly think it's not bad enough to be worth changing. Can you
convince me it's worth it? :-)

Without that change, this would be a four-liner, and I'd review it like a shot...

Gerv

Comment 4

[Mike Kaply [:mkaply]]

That's fine with me.

Can you come up with a good name for the id of the "Other..." resolution selection?

maybe selectResolution...

That was my problem, I was stuck on a name.

Comment 5

[Gervase Markham [:gerv]]

I'm a bit lost; which id exactly are you talking about?

Gerv

Comment 6

[Mike Kaply [:mkaply]]

When you pop down the DPI pull down today, the last option is "Other..." and it
displays the dialog with the line that you are supposed to measure.

In the XUL as it exists today, that menuitem has no id= value.

So I had to come up with one. My brain said that the id should be
"otherResolution" since that what it is. That's the reason I changed the
references to otherResolution to something different so I could name the
"Other..." menuitem with an ID of otherResolution since it made a lot of sense.

Comment 7

[Gervase Markham [:gerv]]

Ah - now I understand. :-) arbitraryResolution or selectResolution are both good.

Gerv

Comment 8

[Mike Kaply [:mkaply]]

Attachment: Much simpler patch that just does the basics

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 108514 [details] [diff] [review]
Much simpler patch that just does the basics

OK, let's try this one :)

Comment 10

[Mike Kaply [:mkaply]]

Comment on attachment 108514 [details] [diff] [review]
Much simpler patch that just does the basics

Bryner:

Could you sr this one for me?

I'd also like to get this one into the phoenix pref-fonts.js/xul as well.

Thanks

Comment 11

[Mike Kaply [:mkaply]]

Attachment: Patch for Phoenix

Phoenix version.

Comment 12

[Mike Kaply [:mkaply]]

Fix in for trunk. Left open to get phoenix fix.

Comment 13

[Mike Kaply [:mkaply]]

Fix checked in to phoenix