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.
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.
QA Contact: sairuh → nobody
Comment on attachment 108103 [details] [diff] [review] Patch Gerv, are you around to take a look at this?
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
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
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
Created attachment 108514 [details] [diff] [review] Much simpler patch that just does the basics
Attachment #108103 - Attachment is obsolete: true
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+
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+
Fix in for trunk. Left open to get phoenix fix.
Fix checked in to phoenix
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.