Remove "other resolution" from DPI for OS/2

RESOLVED FIXED

Status

RESOLVED FIXED
16 years ago
14 years ago

People

(Reporter: mkaply, Assigned: bugs)

Tracking

({qawanted})

Trunk
Other
OS/2
qawanted

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

16 years ago
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.
(Reporter)

Comment 1

16 years ago
Created attachment 108103 [details] [diff] [review]
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.
Keywords: qawanted
QA Contact: sairuh → nobody
(Reporter)

Comment 2

16 years ago
Comment on attachment 108103 [details] [diff] [review]
Patch

Gerv, are you around to take a look at this?
Attachment #108103 - Flags: review?(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
(Reporter)

Comment 4

16 years ago
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.
I'm a bit lost; which id exactly are you talking about?

Gerv
(Reporter)

Comment 6

16 years ago
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.
Ah - now I understand. :-) arbitraryResolution or selectResolution are both good.

Gerv
(Reporter)

Updated

16 years ago
Attachment #108103 - Flags: review?(gerv)
(Reporter)

Comment 8

16 years ago
Created attachment 108514 [details] [diff] [review]
Much simpler patch that just does the basics
Attachment #108103 - Attachment is obsolete: true
(Reporter)

Comment 9

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

OK, let's try this one :)
Attachment #108514 - Flags: review?(gerv)
Attachment #108514 - Flags: review?(gerv) → review+
(Reporter)

Comment 10

16 years ago
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
Attachment #108514 - Flags: superreview?(bryner)
Attachment #108514 - Flags: superreview?(bryner) → superreview+
(Reporter)

Comment 11

16 years ago
Created attachment 109816 [details] [diff] [review]
Patch for Phoenix

Phoenix version.
(Reporter)

Comment 12

16 years ago
Fix in for trunk. Left open to get phoenix fix.
(Reporter)

Comment 13

16 years ago
Fix checked in to phoenix
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.