Last Comment Bug 665342 - In the sync setup window, delete the border between upper and lower sections, and change background color of lower area
: In the sync setup window, delete the border between upper and lower sections,...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All All
: -- minor with 1 vote (vote)
: Firefox 14
Assigned To: Erin W. McCall
:
Mentors:
: 708073 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-19 04:19 PDT by Michael Monreal [:monreal]
Modified: 2013-03-21 07:18 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot (64.87 KB, image/png)
2011-06-19 04:19 PDT, Michael Monreal [:monreal]
no flags Details
Suggested patch for bug (1.87 KB, patch)
2012-03-11 18:27 PDT, Erin W. McCall
dao+bmo: review-
Details | Diff | Review
Removed border and changed lower area color to suggested value (1.81 KB, patch)
2012-03-14 09:23 PDT, Erin W. McCall
dao+bmo: review-
Details | Diff | Review
Updated changes to reflect mozilla-central contents. (1.81 KB, patch)
2012-03-14 10:21 PDT, Erin W. McCall
dao+bmo: review+
Details | Diff | Review

Description Michael Monreal [:monreal] 2011-06-19 04:19:20 PDT
Created attachment 540313 [details]
Screenshot

The bottom of the sync setup window is blue (-> Screenshot), it should be gray (=bgcolor) like the upper region.
Comment 1 Erin W. McCall 2012-03-11 18:27:35 PDT
Created attachment 604812 [details] [diff] [review]
Suggested patch for bug

This is my first patch attempt and I would greatly appreciate any & all feedback!
Comment 2 Dão Gottwald [:dao] 2012-03-13 04:10:09 PDT
Comment on attachment 604812 [details] [diff] [review]
Suggested patch for bug

> .wizard-buttons {
>-  border-top: 2px solid #ccd9ea;
>-  background-color: #f1f5fb;
>+  border-top: 2px solid #990099;

That's quite an ugly color, I don't think we can use that. ;-)

Also, looks like the button should be 1px thick or removed entirely.

>+  background-color: Window;

How about something like rgba(0,0,0,0.1)?
Comment 3 Erin W. McCall 2012-03-13 14:27:20 PDT
(In reply to Dão Gottwald [:dao] from comment #2)
> Comment on attachment 604812 [details] [diff] [review]
> Suggested patch for bug
> 
> > .wizard-buttons {
> >-  border-top: 2px solid #ccd9ea;
> >-  background-color: #f1f5fb;
> >+  border-top: 2px solid #990099;
> 
> That's quite an ugly color, I don't think we can use that. ;-)

Oh my. How embarrassing. I was just playing around making sure I was changing the correct thing and meant to change that back to original color. Will fix and resubmit patch!

> Also, looks like the button should be 1px thick or removed entirely.
> 
> >+  background-color: Window;
> 
> How about something like rgba(0,0,0,0.1)?

Not quite sure what you mean here about the button looking like it should be 1px thick or removed...
Comment 4 Dão Gottwald [:dao] 2012-03-13 14:39:18 PDT
I meant "border" despite writing "button"...
Comment 5 Stephen Horlander [:shorlander] 2012-03-14 07:03:23 PDT
(In reply to Dão Gottwald [:dao] from comment #2)
> How about something like rgba(0,0,0,0.1)?

That is probably the right color for Mac and Linux. The blue for Windows was intentional I think.

(In reply to Dão Gottwald [:dao] from comment #4)
> I meant "border" despite writing "button"...

Yeah I believe the original design was for a 1px border and a slight 1px inset shadow. Not sure how we ended up with a double border :)
Comment 6 Dão Gottwald [:dao] 2012-03-14 07:07:03 PDT
(In reply to Stephen Horlander from comment #5)
> (In reply to Dão Gottwald [:dao] from comment #2)
> > How about something like rgba(0,0,0,0.1)?
> 
> That is probably the right color for Mac and Linux. The blue for Windows was
> intentional I think.

To some extent, yes; it doesn't make sense for all Windows versions and flavours and the border is messed up, as you mentioned. So I'd support just ripping it out across the board unless someone wants to do it the right way.
Comment 7 Stephen Horlander [:shorlander] 2012-03-14 07:08:46 PDT
(In reply to Dão Gottwald [:dao] from comment #6)
> (In reply to Stephen Horlander from comment #5)
> > (In reply to Dão Gottwald [:dao] from comment #2)
> > > How about something like rgba(0,0,0,0.1)?
> > 
> > That is probably the right color for Mac and Linux. The blue for Windows was
> > intentional I think.
> 
> To some extent, yes; it doesn't make sense for all Windows versions and
> flavours and the border is messed up, as you mentioned. So I'd support just
> ripping it out across the board unless someone wants to do it the right way.

That works for me.
Comment 8 Erin W. McCall 2012-03-14 09:23:19 PDT
Created attachment 605797 [details] [diff] [review]
Removed border and changed lower area color to suggested value

I removed the border completely and changed the lower area background color to the suggested rgba(0,0,0,0.1). I don't think this is the "final answer" though..... still looks kind of ugly, just not quite as bad. Any other suggestions to improve the look of this thing? Maybe changing the color to match the upper region and add a super thin plain ol' black border?
Comment 9 Dão Gottwald [:dao] 2012-03-14 09:27:46 PDT
Comment on attachment 605797 [details] [diff] [review]
Removed border and changed lower area color to suggested value

This patch applies on top of the previous patch. Instead, we need a patch that applies on mozilla-central tip.

Otherwise this seems fine, further improvements can happen in new bugs.
Comment 10 Erin W. McCall 2012-03-14 09:32:59 PDT
(In reply to Dão Gottwald [:dao] from comment #9)
> Comment on attachment 605797 [details] [diff] [review]
> Removed border and changed lower area color to suggested value
> 
> This patch applies on top of the previous patch. Instead, we need a patch
> that applies on mozilla-central tip.

Apologies again. I don't understand what you mean by "applies on top of the previous patch" & "applies on mozilla-central tip"?
Comment 11 Dão Gottwald [:dao] 2012-03-14 09:47:14 PDT
The patch tries to remove this line, for instance:

  border-top: 2px solid #990099;

Such a line doesn't exist on mozilla-central, see <http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/syncSetup.css>.
Comment 12 Erin W. McCall 2012-03-14 10:08:00 PDT
(In reply to Dão Gottwald [:dao] from comment #11)
> The patch tries to remove this line, for instance:
> 
>   border-top: 2px solid #990099;
> 
> Such a line doesn't exist on mozilla-central, see
> <http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/
> syncSetup.css>.

Ohhhhhhhhhh. Got it. Ok. Will fix.
Comment 13 Erin W. McCall 2012-03-14 10:21:26 PDT
Created attachment 605831 [details] [diff] [review]
Updated changes to reflect mozilla-central contents.

This patch reflects what is in mozilla-central instead of the previous changes to my working copy.
Comment 14 Dão Gottwald [:dao] 2012-03-14 16:32:12 PDT
Comment on attachment 605831 [details] [diff] [review]
Updated changes to reflect mozilla-central contents.

Thanks!
Comment 16 Dão Gottwald [:dao] 2012-03-14 17:02:36 PDT
Landed a second time with the correct bug number in the commit message.

http://hg.mozilla.org/integration/mozilla-inbound/rev/e36239fc97af
Comment 17 Marco Bonardo [::mak] 2012-03-15 08:25:12 PDT
https://hg.mozilla.org/mozilla-central/rev/e36239fc97af
Comment 18 (Away 6/25-7/4) Jared Wein [:jaws] (reviews and needinfo disabled until back) 2013-03-21 07:18:10 PDT
*** Bug 708073 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.