Closed Bug 431644 Opened 16 years ago Closed 14 years ago

Make resizer in Page Info media tab invisible

Categories

(Firefox :: Theme, defect)

defect
Not set
trivial

Tracking

()

RESOLVED FIXED
Firefox 4.0b3

People

(Reporter: 427384, Assigned: dao)

Details

(Keywords: polish, Whiteboard: [polish-easy] [polish-visual][polish-p3][good first bug])

Attachments

(2 files, 4 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008043007 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008043007 Minefield/3.0pre

There is an unnecessary black line in the Page Info media tab. A dividing line is already present.

Reproducible: Always
Confirmed, I see this using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050105 Minefield/3.0pre.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: polish
Hardware: PC → All
Whiteboard: [polish-easy] [polish-visual]
Version: unspecified → Trunk
The line is the bottom line of the re-sizer, and effects all platforms.  There is no need to have both a resizer and a scroll bar, so we should simply remove the re-sizer widget entirely.  The preview area needs to be fixed so that it doesn't push into the list of images.  Adjusting bug summary.
Summary: Black line in Page Info media tab → Remove resizer in Page Info media tab
Attached patch Patch v.1 (obsolete) — Splinter Review
Assignee: nobody → dolske
Status: NEW → ASSIGNED
Attachment #350610 - Flags: review?(gavin.sharp)
Attached image Screenshot (patch v.1) (obsolete) —
Attachment #350613 - Flags: ui-review?(faaborg)
Comment on attachment 350610 [details] [diff] [review]
Patch v.1

I've found this splitter useful in the past to reduce the amount of scrolling needed when the list was long and I wanted to find one image among many. I don't feel too strongly about it, but it seems like leaving it but fixing the style might be a better alternative? r=me on this if Alex isn't convinced :)
Attachment #350610 - Flags: review?(gavin.sharp) → review+
Attachment #350613 - Flags: ui-review?(faaborg) → ui-review+
An invisible resizer (similar to what we do on Windows between the location bar and search bar) would be fine.

Overall this whole tab feels like a developer tool though, and ultimately I think we need to rethink even having a Page Info window to expose to mainstream users.
This bug's priority relative to the set of other polish bugs is:
P3 - Polish issue that is in a secondary interface, occasionally encountered, or is not easily identifiable.

Page info is a secondary UI, and the change will likely be imperceptible to most users.
Whiteboard: [polish-easy] [polish-visual] → [polish-easy] [polish-visual][polish-p3]
Component: Theme → Page Info
why the bug is still not fixed? it got patch, patch got review+ >1 year ago. What's up?
It's not checked in because I agree with Gavin's comment 6. Instead of removing the resizer, better to make it invisible (or remove the duplicate line instead). Currently this looks fine on OS X, but Windows shows the double-line effect.

I don't have time to follow up on this right now, someone else should feel free to pick up.
Assignee: dolske → nobody
Status: ASSIGNED → NEW
Whiteboard: [polish-easy] [polish-visual][polish-p3] → [polish-easy] [polish-visual][polish-p3][good first bug]
I'll get a patch in the next few days.
Assignee: nobody → mozilla.bugs
Status: NEW → ASSIGNED
(In reply to comment #10)
> It's not checked in because I agree with Gavin's comment 6. Instead of removing
> the resizer, better to make it invisible (or remove the duplicate line
> instead). Currently this looks fine on OS X, but Windows shows the double-line
> effect.

I am opting for keeping the resizer but making it invisible; so should the patch be Windows-specific since it's on Windows where the problem is the worst?
No, the splitter is certainly visible on linux as well, so we may as well include it. I don't have a mac handy, but I imagine it's much the same there. Once you've made the change in the windows theme it won't be hard to do the others; very likely it'll be precisely the same change, merely in a different directory.
Excellent, I am now only waiting on a Firefox crash that's preventing me from working before I get to this, and it should land in the next two or three days.
QA Contact: theme → page.info
Component: Page Info → Theme
QA Contact: page.info → theme
Summary: Remove resizer in Page Info media tab → Make resizer in Page Info media tab invisible
Attachment #350613 - Attachment is obsolete: true
Attachment #350610 - Attachment is obsolete: true
Attached patch Patch v. 1.0 (obsolete) — Splinter Review
Attachment #458116 - Flags: review?(dao)
Attachment #458116 - Attachment is patch: true
Attachment #458116 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 458116 [details] [diff] [review]
Patch v. 1.0

>+++ b/browser/themes/gnomestripe/browser/pageInfo.css

> /* Media Tab */
> #imagetree {
>   min-height: 10em;
>   margin-top: 2px;
>+  margin-bottom: 0em;

just "0"

>+#mediaSplitter {
>+  border-top-width: 0px;
>+  border-bottom-width: 0px;

this doesn't seem to be in line with http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/gnomestripe/global/splitter.css

>+++ b/browser/themes/pinstripe/browser/pageInfo.css

>+#mediaSplitter {
>+  border-top-width: 0px;
>+  border-bottom-width: 0px;

this doesn't seem to be in line with http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/pinstripe/global/splitter.css

>+++ b/browser/themes/winstripe/browser/pageInfo.css

>+#mediaSplitter {
>+  border-top-width: 0px;
>+  border-bottom-width: 0px;

border-style: none;

Also, to be in line with http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/splitter.css you should add background: none;.
Attachment #458116 - Flags: review?(dao) → review-
Due to time constraints, I can only work on the Windows element of this bug.  I will have a patch for that today, but I don't have the time or resources to test and develop any changes in other OSes.
Attached patch Patch v 1.1 (obsolete) — Splinter Review
Windows-only fix.
Attachment #458116 - Attachment is obsolete: true
Attachment #459227 - Flags: review?(dao)
I'll take this over then to fix it across platforms.
Assignee: mozilla.bugs → dao
Attachment #459227 - Flags: review?(dao)
Attached patch patchSplinter Review
Attachment #459227 - Attachment is obsolete: true
Attachment #459814 - Flags: review?(gavin.sharp)
Attachment #459814 - Flags: review?(gavin.sharp) → review+
Attachment #459814 - Flags: approval2.0?
Attachment #459814 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/e226c2600723
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b3
You need to log in before you can comment on or make changes to this bug.