Last Comment Bug 690870 - [TABLETUI] Add white horizontal divider after first group of Sync JPAKE characters
: [TABLETUI] Add white horizontal divider after first group of Sync JPAKE chara...
Status: VERIFIED FIXED
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Firefox 9
: ARM Android
: -- normal (vote)
: Firefox 9
Assigned To: Lucas Rocha (:lucasr)
:
Mentors:
Depends on:
Blocks: 655762 682412 688168
  Show dependency treegraph
 
Reported: 2011-09-30 12:15 PDT by Aaron Train [:aaronmt]
Modified: 2013-12-10 10:00 PST (History)
7 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Nightly (09/30) (14.21 KB, image/png)
2011-09-30 12:15 PDT, Aaron Train [:aaronmt]
no flags Details
Add missing separator between sync code numbers (877 bytes, patch)
2011-10-03 05:50 PDT, Lucas Rocha (:lucasr)
mark.finkle: review-
Details | Diff | Review
Bump magin_xtiny from 0.11mozmm to 0.15mozmm (1.05 KB, patch)
2011-10-03 12:29 PDT, Lucas Rocha (:lucasr)
mark.finkle: review+
asa: approval‑mozilla‑aurora+
Details | Diff | Review

Description Aaron Train [:aaronmt] 2011-09-30 12:15:25 PDT
Created attachment 563801 [details]
Nightly (09/30)

Remove the white horizontal divider above the last group of Sync JPAKE characters

--
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20110930 Firefox/10.0a1 Fennec/10.0a1
Galaxy Tab 10.1 (Android 3.1)
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-09-30 12:25:30 PDT
Do we want to remove the white line or add a white line between the first and second boxes?

I would have thought we'd add a white separator
Comment 2 Ian Barlow (:ibarlow) 2011-09-30 12:31:17 PDT
there should be two white separators

http://www.flickr.com/photos/61892693@N03/6169097025/in/photostream
Comment 3 Lucas Rocha (:lucasr) 2011-10-03 05:50:27 PDT
Created attachment 564167 [details] [diff] [review]
Add missing separator between sync code numbers

This patch works fine but, to be honest, I'm not entirely sure why. The first description element in the sync code vbox is getting an tiny extra margin somehow. I couldn't find the corresponding CSS selector that is causing that. Forcing the margin of first description to be 0 fixes the issue.
Comment 4 Lucas Rocha (:lucasr) 2011-10-03 06:21:35 PDT
FYI: the DOM inspector didn't show anything interesting regarding this. All description elements with .syncsetup-code class have the same computed style.
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-03 07:45:24 PDT
Comment on attachment 564167 [details] [diff] [review]
Add missing separator between sync code numbers

Is this something we need to add to _all_ descriptions?
Comment 6 Lucas Rocha (:lucasr) 2011-10-03 07:55:21 PDT
(In reply to Mark Finkle (:mfinkle) from comment #5)
> Comment on attachment 564167 [details] [diff] [review] [diff] [details] [review]
> Add missing separator between sync code numbers
> 
> Is this something we need to add to _all_ descriptions?

This patch removes 1-2px margin that only applies to the first-child description. I wonder why we'd want to keep this strange extra margin for other description elements.

I can move this fix to apply only to syncsetup-code class if you feel strongly about it.
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-03 07:58:07 PDT
I think this margin is the problem:
http://mxr.mozilla.org/mozilla-central/source/mobile/themes/core/honeycomb/browser.css#1795

margin_xtiny = 0.11mozmmm and I think that is too small. It could be causing a rounding error. Let's try manually bumping up that margin a bit to see if we get the desired affect without rounding issues.
Comment 8 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-03 07:59:07 PDT
Comment on attachment 564167 [details] [diff] [review]
Add missing separator between sync code numbers

I feel strongly that we don't want this rule in platform.css
Comment 9 Lucas Rocha (:lucasr) 2011-10-03 12:29:28 PDT
Created attachment 564292 [details] [diff] [review]
Bump magin_xtiny from 0.11mozmm to 0.15mozmm

You're right, bumping margin_xtiny to 0.15mozmm fixes the problem. margin_xtiny is not used in many places. This bump doesn't seem to break anything.
Comment 10 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-03 12:34:58 PDT
Comment on attachment 564292 [details] [diff] [review]
Bump magin_xtiny from 0.11mozmm to 0.15mozmm

Please update froyo and gingerbread margin_xtiny too
Comment 11 Matt Brubeck (:mbrubeck) 2011-10-04 14:35:12 PDT
This bug is currently not in Firefox 9, but we should check whether it appears on Aurora after we land other theme patches there, and backport this fix too if it does.
Comment 12 Lucas Rocha (:lucasr) 2011-10-05 03:02:22 PDT
Landed: http://hg.mozilla.org/integration/mozilla-inbound/rev/b6d4674960af
Comment 13 Marco Bonardo [::mak] 2011-10-05 05:15:48 PDT
https://hg.mozilla.org/mozilla-central/rev/b6d4674960af
Comment 14 Matt Brubeck (:mbrubeck) 2011-10-06 06:31:25 PDT
Since bug 682412 landed on Aurora, this has regressed there too.
Comment 15 Matt Brubeck (:mbrubeck) 2011-10-06 06:35:07 PDT
Comment on attachment 564292 [details] [diff] [review]
Bump magin_xtiny from 0.11mozmm to 0.15mozmm

Requesting approval for Aurora 9. This mobile-only patch just changes one constant in our CSS to avoid rounding errors, to fix a regression that makes the sync dialog look ugly. The fix has already been in nightly for several days.
Comment 16 Aaron Train [:aaronmt] 2011-10-07 06:50:39 PDT
Verified fixed on Nightly
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111007 Firefox/10.0a1 Fennec/10.0a1
Comment 17 Lucas Rocha (:lucasr) 2011-10-07 09:33:04 PDT
Landed in Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/b2f1ffa12cd2
Comment 18 Camelia Urian 2011-10-14 06:02:05 PDT
Verified fixed on Aurora
Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20111013 Firefox/9.0a2 Fennec/9.0a2

White horizontal divider exists after first and second group of Sync JPAKE characters.

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