Closed Bug 149448 Opened 22 years ago Closed 20 years ago

Themes with Long Descriptions Scroll "Get New Themes" Out of View

Categories

(Core Graveyard :: Skinability, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: unmobile+mozbugs, Assigned: durbacher)

References

Details

(Whiteboard: DUPEME)

Attachments

(3 files, 4 obsolete files)

If a theme were created with an excessively long description, the link at the
bottom of the Themes pref-panel would no longer be visible. This results from
the "Get New Themes" link being included in the text area used for display of
the description. Note that, when a theme has an excessively long description,
some of the description may no longer be visible either.
This is a modified version of chrome.rdf with the description of the classic
theme repeated numerous times, causing the "Get New Themes" link to be pushed
off-screen when the "Classic" theme is highlighted.
To use, close Mozilla, copy this file over chrome.rdf in the Mozilla\chrome
directory, reopen Mozilla, open the prefs dialog, select Appearance|Themes, and
highlight "Classic".
Expected Result:
"Get New Themes" link remains in a fixed position, maintaining UI consistency.
Actual Result:
"Get New Themes" link is pushed down by the long description text, to the point
that it is no longer visible to the user.
Bug 149543 has been created to cover the related issue of part of the
description becoming non-visible as well. However, fixing this bug only requires
moving the "Get New Themes" link out of the description text area, while fixing
that one requires adding some provision for extending that area beyond the size
of the prefs panel.
Confirming on 1.0RC3, Windows 98.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 149453
This is a dupe (possible twice submitted) and this bug belongs to skinability
(since the pref panel is from there)

I can's search at the moment because the bug databse is laggy.
Whiteboard: DUPEME
--> Skinability
Component: Preferences → Skinability
*** Bug 160974 has been marked as a duplicate of this bug. ***
Makes the description scrollable when it does not fit in the dialog (with my
current default Win settings this happens as soon as there are more than three
description lines).
Assignee: bugs → durbacher
Status: NEW → ASSIGNED
Comment on attachment 140424 [details] [diff] [review]
patch, show scrollbar for too long descriptions.

Requesting r= from Neil.

(BTW, this will also fix bug 149453, of course, which is almost - if not really
- a duplicate).
Attachment #140424 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 140424 [details] [diff] [review]
patch, show scrollbar for too long descriptions.

There are two things that I don't like with this solution (although that
doesn't mean there's a better solution) - the first is that the margin is on
the label, not the vbox, so that the scrollbar doesn't line up with the other
items in the panel, the second is that because the flex normally pushes the
link to the bottom of the panel you don't need a separator, or at least not so
large a separator.
Attached patch improved patch (obsolete) — Splinter Review
This patch replaces the separator below by a class="thin" one; the gap between
is now smaller and now four lines do still fit without a scrollbar appearing.

I added a right margin of 5px to the vbox so the scrollbar lines up with the
items below and above. This does work across themes, I also tested Orbit and
LCARStrek.
Margins of the other elements are partly taken from formatting.css, e.g.
.description and .inset (for the preview image) get 5px from there for both
themes in lxr and for both mac/win (I think unix uses win). So those 5 px are
pretty safe to be correct.
Attachment #140424 - Attachment is obsolete: true
Attachment #140424 - Flags: review?(neil.parkwaycc.co.uk)
Alternatively use class="inset" which gets the margin from the current theme
and should be perfectly safe this way.
It also changes the appearance of the description to look "deeper", which looks
especially nice if there is a scrollbar (what should happen rarely, though).
Not sure if this is desired. I'll attach screenshots.
Attached image screenshot for "improved patch" (obsolete) —
Comment on attachment 140505 [details] [diff] [review]
improved patch using class="inset"

Setting r= request for Neil; however, please consider both current patches
because I'm not sure if that inset is ok for you...
Attachment #140505 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 140505 [details] [diff] [review]
improved patch using class="inset"

Hey, that looks great. Do we still need a separator with class="inset"?
Attachment #140505 - Flags: review?(neil.parkwaycc.co.uk) → review+
No, we don't. This patch has the separator removed and it still looks good
(distance between is a tiny bit smaller, but still fine).
Attachment #140504 - Attachment is obsolete: true
Attachment #140505 - Attachment is obsolete: true
Attachment #140506 - Attachment is obsolete: true
Comment on attachment 140552 [details] [diff] [review]
patch with inset and no separator

Again requesting r= from Neil.
Attachment #140552 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #140552 - Flags: review?(neil.parkwaycc.co.uk) → review+
Comment on attachment 140552 [details] [diff] [review]
patch with inset and no separator

Requesting sr= from alecf.
Attachment #140552 - Flags: superreview?(alecf)
Comment on attachment 140552 [details] [diff] [review]
patch with inset and no separator

sr=alecf
Attachment #140552 - Flags: superreview?(alecf) → superreview+
Checking in xpfe/components/prefwindow/resources/content/pref-themes.xul;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-themes.xul,v
 <--  pref-themes.xul
new revision: 1.51; previous revision: 1.50
done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
verified with current cvs build
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: