Last Comment Bug 783267 - Box.com auth window spinner is misaligned
: Box.com auth window spinner is misaligned
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: FileLink (show other bugs)
: 15 Branch
: x86 Windows 7
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Richard Marti (:Paenglab)
:
:
Mentors:
Depends on:
Blocks: 767327
  Show dependency treegraph
 
Reported: 2012-08-16 07:26 PDT by Mike Conley (:mconley) - Getting through review / needinfo backlog
Modified: 2012-08-27 06:45 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
Misaligned spinner (10.65 KB, image/png)
2012-08-16 07:27 PDT, Mike Conley (:mconley) - Getting through review / needinfo backlog
no flags Details
Properly aligned icon for reference (16.27 KB, image/png)
2012-08-16 07:27 PDT, Mike Conley (:mconley) - Getting through review / needinfo backlog
no flags Details
patch (1.48 KB, patch)
2012-08-16 08:28 PDT, Richard Marti (:Paenglab)
mconley: review+
bwinton: approval‑comm‑aurora+
Details | Diff | Splinter Review
Still misaligned (13.15 KB, image/png)
2012-08-17 07:04 PDT, Mike Conley (:mconley) - Getting through review / needinfo backlog
no flags Details

Description User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-16 07:26:37 PDT
When the Box.com auth window first comes up, and token negotiation begins, the spinner appears to be misaligned (at least on Windows).

See screenshots.
Comment 1 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-16 07:27:03 PDT
Created attachment 652445 [details]
Misaligned spinner
Comment 2 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-16 07:27:20 PDT
Created attachment 652446 [details]
Properly aligned icon for reference
Comment 3 User image Richard Marti (:Paenglab) 2012-08-16 08:28:38 PDT
Created attachment 652461 [details] [diff] [review]
patch

Instead of playing with margins it's better to use background-position. Then also the URL doesn't jump.
Comment 4 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-16 12:41:55 PDT
Comment on attachment 652461 [details] [diff] [review]
patch

Review of attachment 652461 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't built with this to see what these look like, but I trust your eye. r=me! Thanks Richard!
Comment 5 User image Brian King [:kinger] 2012-08-16 14:20:30 PDT
FYI the root of this bug is that the spinner icon is 16x16 which is smaller than the 22x22 size of the security level icon(s), hence the need for it to be positioned so that the transition between icons does not cause too much of a visual jump.
Comment 6 User image Ryan VanderMeulen [:RyanVM] 2012-08-16 17:43:40 PDT
https://hg.mozilla.org/comm-central/rev/caf3e0f5733f
Comment 7 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-17 07:03:57 PDT
Hrm - I probably should have tried the patch before giving it my r+. The spinner is still misaligned in today's Daily.

See upcoming screenshot.
Comment 8 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-17 07:04:40 PDT
Created attachment 652755 [details]
Still misaligned
Comment 9 User image Richard Marti (:Paenglab) 2012-08-17 07:19:58 PDT
Hmm, I checked with my updated Daily and the change isn't in it :/ Still using the margin.

Could this be because of the DONTBUILD on check-in?
Comment 10 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-17 07:32:56 PDT
Turns out I didn't see the fix because this patch didn't land until *after* today's Daily was built. *sigh*.

Sorry about that. Local testing confirms that this patch is indeed awesome.
Comment 11 User image Brian King [:kinger] 2012-08-24 09:19:50 PDT
Comment on attachment 652461 [details] [diff] [review]
patch

[Approval Request Comment]
Regression caused by (bug #): 767327
User impact if declined: UX snafu
Testing completed (on c-c, etc.): ? 
Risk to taking this patch (and alternatives if risky): Small. Dependent on approval‑comm‑aurora of bug 767327
Comment 12 User image Blake Winton (:bwinton) (:☕️) 2012-08-27 06:25:31 PDT
Comment on attachment 652461 [details] [diff] [review]
patch

a=me, since it seems small and innocuous, and lining things up is one of my crazy-making things…  ;)
Comment 13 User image Mike Conley (:mconley) - Getting through review / needinfo backlog 2012-08-27 06:45:15 PDT
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/d6967732c4f0

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