1132194 - [RTL][Contacts] The + icon overlaps the title for the associated 'add' field (example: Add Email, Add Phone).

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect, P2)

Description

[Joshua Mitchell (Inactive)]

Attachment: 2015-02-11-13-11-33.png

Description:
In contacts app, when editing or creating a new contacts, you are given an option of adding additional information fields. This is shown by a + icon next to the words "add ....."
These icons and words overlap.
This affects add: Phone, Email, Address, Date, Comment.


Repro Steps:
1) Update a Flame to 20150211010216
2) Launch Contacts app
3) Select + to add new contact
4) Observe + icons on contact detail / edit page

Actual:
+ icons overlap with text

Expected:
icons will not overlap text

Environmental Variables:
Device: Flame 3.0
Build ID: 20150211010216
Gaia: 8c7865486a1b11076b849bbf8f7fccbaffbfafe7
Gecko: ee093ca70666
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency:5/5
See attached: screenshot 

------------------------------------------------------------------------------

Issue does NOT reproduce in 2.1

Actual results - icons do not overlap 

Device: Flame 2.2 (KK - Nightly - Full Flash)
Build ID: 20150211002505
Gaia: 943be6fd146017dcd9d4c9d1027be1e43bad13eb
Gecko: e614443583e7
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 1

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Identifiable regression of a supported feature.

Requesting a window.

Comment 2

[Yeojin Chung [:YeojinC]]

Mozilla-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150209063429
Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec
Gecko: c13e7799a147
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150209064732
Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec
Gecko: 70a1847bdc20
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec
Gecko: 70a1847bdc20

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 0d7b35f23402c4cb29bca6b98280fec48a196dec
Gecko: c13e7799a147

Gecko Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c13e7799a147&tochange=70a1847bdc20
=================================

Caused by Bug 1130231.

Comment 3

[KTucker [:KTucker][Inactive 3/4/2016]]

Jonathan, can you look at this please? Looks like the work done on Bug 1130231 might have caused this.

Comment 4

[Jonathan Kew [:jfkthame]]

Michael, it looks like the patch from bug 1130231 is having unexpected(?) effects in some places... I'm wondering if this is in fact a Gecko layout bug, or if it's caused by a lack of correct RTL-awareness in some of the Gaia CSS, but things happened to look OK previously thanks to the padding bug that we fixed in 1130231.

Any chance you could isolate a minimal example of the issue here, so we can determine whether it needs to be fixed in the platform or in the apps? That would be really helpful. Thanks!

Comment 5

[Michael Henretty [:mikehenrty][:mhenretty]]

It's highly possible we were relying on the bug to display RTL correctly. I'll take a look at this later tonight or tomorrow. Leaving ni?.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] jfkthame:bug-1132194 > mozilla-b2g:master

Comment 7

[Jonathan Kew [:jfkthame]]

AFAICT, this should fix the Contacts issue; but note that it involves touching the shared buttons.css, so we need to be alert for anything else that may be affected by that.

In general, I'm seeing a lot of Gaia CSS where margins, padding, etc are specified with physical properties, and then there are separate rules appended to the files that use a dir=rtl selector to provide the "reversed" layout for RTL. I think it'd be better to replace this pattern, as far as possible, with use of -start/-end properties that will automatically handle the direction; this should reduce the overall number of rules needed, and provide a performance improvement (fewer selectors to match).

Comment 8

[Dylan Oliver [:doliver]]

triage: not marking for feature-b2g:2.2 at this time because it's marked as unaffected, but if this does show up on 2.2, please request approval for uplift.

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Jonathan Kew (:jfkthame) from comment #7)
> In general, I'm seeing a lot of Gaia CSS where margins, padding, etc are
> specified with physical properties, and then there are separate rules
> appended to the files that use a dir=rtl selector to provide the "reversed"
> layout for RTL. I think it'd be better to replace this pattern, as far as
> possible, with use of -start/-end properties that will automatically handle
> the direction; this should reduce the overall number of rules needed, and
> provide a performance improvement (fewer selectors to match).

Agreed. I think the reason we have a lot of html[dir="rtl"] selectors is that when we started this effort last year we weren't aware we had some of these features like -moz-padding-start. I believe people are actively trying to clean this up.

Comment 10

[Michael Henretty [:mikehenrty][:mhenretty]]

(In reply to Dylan Oliver [:doliver] from comment #8)
> triage: not marking for feature-b2g:2.2 at this time because it's marked as
> unaffected, but if this does show up on 2.2, please request approval for
> uplift.

Dylan, we need to mark this feature-b2g:2.2. The only reason it was marked as not affecting 2.2 was because bug 1130231 hadn't been uplifted to ff37 at the time of this filing [1].

1.) https://bugzilla.mozilla.org/show_bug.cgi?id=1130231#c17

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8563439 [details] [review]
[gaia] jfkthame:bug-1132194 > mozilla-b2g:master

Ok, I tried to check as many applications that I could which use buttons (calendar, cost control, clock, contacts, camera, etc) and I didn't see any problems in the button padding. We should be wary though for any regressions here, but I think in general this patch will work.

Kevin, what do you think?

Comment 12

[Kevin Grandon :kgrandon]

Comment on attachment 8563439 [details] [review]
[gaia] jfkthame:bug-1132194 > mozilla-b2g:master

Makes sense to me.

Comment 13

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 14

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8563439 [details] [review]
[gaia] jfkthame:bug-1132194 > mozilla-b2g:master

Francisco, you are a contacts peers, can you check this patch?

Comment 15

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8563439 [details] [review]
[gaia] jfkthame:bug-1132194 > mozilla-b2g:master

Francisco is a bit loaded this week, forwarding.

Comment 16

[Jose Manuel Cantera]

Comment on attachment 8563439 [details] [review]
[gaia] jfkthame:bug-1132194 > mozilla-b2g:master

LGTM

thanks

Comment 17

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/96b1979aecdf36b92fb2426b723ac29e4fbe8d7e

Comment 18

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8563439 [details] [review]
[gaia] jfkthame:bug-1132194 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Bug 1130231

[User impact] if declined:
UI hard to read on the contacts edit form.

[Testing completed]:
Manually tested.

[Risk to taking this patch] (and alternatives if risky):
No alternative. We need this css patch if we want this fixed. Low risk in any case.

[String changes made]: none.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/b7fb3f31ff647c4a3a2f0c9aa7d194837ae292b1

Comment 20

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is fixed in the latest Nightly 3.0 and 2.2 Flame builds.  

Results:  The text in RTL languages does not overlap the "+" icon.

Environmental Variables:
Device: Flame 3.0 KK (319MB) (Full Flash)
BuildID: 20150219010228
Gaia: 620aecfde85a8b093247837c55de2708e22be1e1
Gecko: 360b5f211180
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Environmental Variables:
Device: Flame 2.2 KK (319MB) (Full Flash)
BuildID: 20150219002504
Gaia: ce79d35b92261e7cbfeaefebf87859ebeb0979b4
Gecko: 159a3907b959
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0