Closed Bug 420736 Opened 13 years ago Closed 12 years ago

Part of advanced preferences tab cut off


(Firefox :: Preferences, defect, P2)




Firefox 3.6a1


(Reporter: micmon, Assigned: mkohler)





(2 files, 4 obsolete files)

The right border of the advanced preferences tab is not being displayed. I think the dialog has to be sized a bit bigger for it to fit.
Attached image Screenshot
This helps:

prefwindow {
  width: 520px !important;
Blocks: 411110
Attached patch Patch v1.0 (obsolete) — Splinter Review
Made a patch out of Michael's solution. Thanks, Michael!

Sorry for marking as review? and ui-review? but I didn't know which one I have to take.
Attachment #362065 - Flags: ui-review?
Attachment #362065 - Flags: review?
Attachment #362065 - Flags: ui-review? → ui-review?(beltzner)
Attachment #362065 - Flags: review? → review?(benjamin)
That dialog's width is set in preferences.dtd (see the prefWin.styleGNOME2 entity) and depends on the font used - on the font height, to be precise, as we set the width in em-units.

IIRC we don't automatically calculate the dialog's required dimensions because the individual panes are only loaded on demand. So the simplest solution would be to increase the width to e.g. 44em. Then again, maybe this would be the time for changing to ch-units and adding a min-width (as some UI elements don't scale down as much as fonts do). Not sure if there's a less hackish solution to this issue (which BTW keeps cropping up from time to time)...
Component: Widget: Gtk → Preferences
Product: Core → Firefox
QA Contact: gtk → preferences
Attachment #362065 - Attachment is obsolete: true
Attachment #362065 - Flags: ui-review?(beltzner)
Attachment #362065 - Flags: review?(benjamin)
Comment on attachment 362065 [details] [diff] [review]
Patch v1.0

This patch is wrong in several ways: It sets a width on all prefwindows, even though only a specific one is ill-sized; it adds an !important rule which we try to avoid as often as possible in order not to make theming more difficult than it already is; it adds a magic number without explanation (we've already got too many of those, so let's not add more); and finally, it breaks several locales which need a significantly wider dialog (just try the German one ;-)).

BTW: ui-review is only for significant changes in UI. Obvious bugs only need review.
Attachment #362065 - Attachment is obsolete: false
Attachment #362065 - Attachment is obsolete: true
No longer blocks: 411110
Duplicate of this bug: 411110
Attached patch like that? (Patch v2.0) (obsolete) — Splinter Review
So, next try. A few notes:

I added min-width, but I couldn't test in Windows and in Mac OS. Just tested with Ubuntu 8.10 (Linux).

> +<!ENTITY  prefWin.styleGNOME2     "min-width: 44em; min-height: 40.5em;">

min-width increased by 2, like that the buttons show correctly and aren't cut off.

I also increased min-height by one. If the "advanced" tab opens at startup, the bottom line isn't shown. It just shows up when an other tab, which needs a higher height, gets opened and then you switch back to "advanced". With a value of "40.5em" the bottom line is shown without this hack.
Attachment #362755 - Flags: review?
Attached patch Patch v3.0 (obsolete) — Splinter Review
Attachment #362755 - Attachment is obsolete: true
Attachment #362911 - Flags: review?
Attachment #362755 - Flags: review?
Attachment #362911 - Flags: review? → review?(mano)
forgot to say that it happens also with the content tab.. shall I fill a new bug report?
Attachment #362911 - Flags: review?(mano)
Attached patch Patch v4.0 (obsolete) — Splinter Review

* name of the entity to*
* increased min-width for GNOME again, so the right line border in the "Advanced" tab is shown too

Unfortunately I couldn't test this patch on Mac OS because I have no Mac system here. Could anybody do that for me? (till then it's not marked as "review?")
Attachment #362911 - Attachment is obsolete: true
smaug tested it on Mac OS and it seems to work ok. So marking for review.
Attachment #364535 - Flags: review?(
Assignee: nobody → michaelkohler
Whiteboard: [has patch][needs review gavin]
Priority: -- → P2
Attachment #364535 - Flags: review?( → review+
Has this been tested on all three platforms?
Whiteboard: [has patch][needs review gavin] → [has patch]
yes, so adding [checkin-needed]
Whiteboard: [has patch] → [checkin-needed]
Keywords: checkin-needed
Whiteboard: [checkin-needed]
Attachment #364535 - Flags: approval1.9.1?
Attachment #364535 - Attachment is obsolete: true
Attachment #364535 - Flags: approval1.9.1?
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Comment on attachment 366625 [details] [diff] [review]
merged [Checkin comment 15]
Attachment #366625 - Attachment description: merged (so ready for check-in) → merged [Checkin comment 15]
couple of notes: I believe this is caused by the four buttons on the encryption tab and Mac OS X worked around this issue by putting the last button in its own box below the first three buttons. Also, this affects Windows when using larger fonts than the default. I personally think we should for the time being make the same change as was done for Mac OS X for all platforms and redesign the encryption tab sometime in the future. btw: the prefwindow does calculate height but it doesn't width... I personally think issues with width should be handled either by designing the pref panes with width in mind or providing better methods for wrapping xul elements.
You need to log in before you can comment on or make changes to this bug.