STR: 1. open "New Message" window 2. enter 2 lines of recipients 3. try to slide down the panel to clearly see all recipients => this does not work. (current master) Note that this work as soon as there are 3 lines of recipients. This works on my peak v1.4, so it's a regression, probably from the composer refactoring in bug 963018.

Umm, in today's master build I am able to slide down the panel and see the two lines of recipients.

So maybe something else more intermittent is happening... I think we noticed we couldn't slide down the panel if the recipients come from a draft, so maybe I'm seeing this ?

(In reply to Julien Wajsberg [:julienw] from comment #2) > So maybe something else more intermittent is happening... > > I think we noticed we couldn't slide down the panel if the recipients come > from a draft, so maybe I'm seeing this ? It's not intermittent issue. I've put the STR in bug 1021513 comment 7: 1) Create a multi-recipient message and save as draft (gesture works now) 2) Close message app, and launch again(If we don't close the app, this issue could not happend :-/) 3) Open the draft again, and the gesture could not work(even create another message)

Hi Julien, this patch is my first proposal that: 1) Remove the view dims because I think it's redundant in current recipient structure 2) Use minHeight instead, but maybe you have better idea about how to detect if container should be pannable or not. Since test for this pan gesture part didn't exist before, we will need additionl test for it after we have conclusion, thanks.

Comment on attachment 8440622 [details] [diff] [review] Bug-1022644-Messages-Can-t-open-the-recipient-panel-.patch Review of attachment 8440622 [details] [diff] [review]: ----------------------------------------------------------------- ::: apps/sms/js/recipients.js @@ +740,5 @@ > // 2. The user is "pulling down" the recipient list. > > // #1 > + if (view.state.visible === 'singleline' && > + view.inner.scrollHeight > view.minHeight) { here is another idea: since we scroll the inner container in .focus and .render (using different code by the way), maybe we only need to check that scrollTop > 0? and then we don't need to keep either minHeight or dims.

(In reply to Julien Wajsberg [:julienw] from comment #5) > Comment on attachment 8440622 [details] [diff] [review] > Bug-1022644-Messages-Can-t-open-the-recipient-panel-.patch > > Review of attachment 8440622 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: apps/sms/js/recipients.js > @@ +740,5 @@ > > // 2. The user is "pulling down" the recipient list. > > > > // #1 > > + if (view.state.visible === 'singleline' && > > + view.inner.scrollHeight > view.minHeight) { > > here is another idea: since we scroll the inner container in .focus and > .render (using different code by the way), maybe we only need to check that > scrollTop > 0? > > and then we don't need to keep either minHeight or dims. The scrollTop would alway be 0 until recipient list longer that max container height, container position is controlled by transform, not by scroll position. Another idea is we just show the multiline mode(maybe rename it like "showFullList") anyway no matter the container is only single line or not. Not many user will try to swipe container if there is only single line, but the only problem is we set additional padding bottom for container in multiline mode. If we don't set this padding for multiline mode, there is no difference for sinle line container under 2 modes.

Hi Julien, you said you might have another idea for this case during the scrum. It would be great to have a simpler solution that can get rid of all the computed styling value.

Same issue happens in v1.4 => requesting 1.4 blocker status.

v1.3t has a limited and different version of the draft feature, and has the same issue with this STR: 1. start a new message 2. add more than 2 lines of recipients 3. add some text 4. kill the app 5. restart the app 6. try to slide down the recipient. So nominating for v1.3t too.

Comment on attachment 8440622 [details] [diff] [review] Bug-1022644-Messages-Can-t-open-the-recipient-panel-.patch Review of attachment 8440622 [details] [diff] [review]: ----------------------------------------------------------------- Now that I understand better the issue, I think your solution should work fine. Here are some comments. Of course we'll need unit tests. And since the bug is present in all branches that have drafts, I think we need to move forward quickly here. ::: apps/sms/js/recipients.js @@ +374,4 @@ > var template = setup.template; > var nodes = []; > var clone; > + var outerCss = window.getComputedStyle(outer, null); nit: you don't need the second argument. @@ +387,4 @@ > isTransitioning: false, > visible: 'singleline' > }, > + minHeight: parseInt(outerCss.getPropertyValue('min-height'), 10) Why don't you use "height" here? I'd say the "height" when the app is starting would have the correct value? But "min-height" will keep a constant value so it's maybe safer. In the end, your choice, I'll let you choose !

Ni Bhavana for the nomination, since I don't know if there still are v1.3t triages.

I've blocked this all the way upto 1.3T given comment #9. you will have to self land it on the branch once ready(central landing/testing completed). Here is the branch information : https://wiki.mozilla.org/Release_Management/B2G_Landing#v1.3T For getting this fixed on 1.4/2.0, once it landed on central, please seek approval. Flip the approval‑mozilla‑b2g30 and fill in the attachment, and once approved landings will be taken care. Thanks !

Hi Oleg, this patch basically modified form previous one that Julien already gave some feedback. Could you please take a look when he is OOO? Thanks.

Comment on attachment 8446341 [details] Link to github Don't have anything to add, looks good! :) Just tiny question at github. Thanks!

(In reply to Oleg Zasypkin [:azasypkin] from comment #14) > Comment on attachment 8446341 [details] > Link to github > > Don't have anything to add, looks good! :) Just tiny question at github. > > Thanks! I replied here: https://github.com/mozilla-b2g/gaia/pull/20973/files#r14277609 and I made a little adjustment. Do you think this change is fine?

(In reply to Steve Chung [:steveck] from comment #15) > (In reply to Oleg Zasypkin [:azasypkin] from comment #14) > > Comment on attachment 8446341 [details] > > Link to github > > > > Don't have anything to add, looks good! :) Just tiny question at github. > > > > Thanks! > > I replied here: https://github.com/mozilla-b2g/gaia/pull/20973/files#﷒0﷓ and > I made a little adjustment. Do you think this change is fine? Yes, I think it's fine!

in master: b04d61c36ca7e0c0a949dbb1bf506b1220082c93 Will uplift this patch for v1.3T later.

Hi Oleg, since the structure of recipient is different in v1.3t, I adjusted the patch a liitle bit for 1.3t branch. Could you please review it again? Thanks.

Comment on attachment 8447930 [details] Link to github(for v1.3t) r=me, just left small nit at GitHub. Thanks!

Comments addressed and Travis is green now, thanks! on v1.3t: 366bf651ac320c8ccc458f08ece01f2b8aadfa1c

Steve, you still need to raise approvals for 1.4 and 2.0 here. I guess you can ask approval for the 2.0 on the master patch, and approval for 1.4 on the 1.3t patch.

(In reply to Julien Wajsberg [:julienw] (away June 21 to 30) from comment #21) > Steve, you still need to raise approvals for 1.4 and 2.0 here. > > I guess you can ask approval for the 2.0 on the master patch, and approval > for 1.4 on the 1.3t patch. Ya I know, just verifying the uplifting works(got a small conflict when uplift to v1.4, will create another patch for that).

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Regression from message draft feature. [User impact] if declined: User could not drag down the recipient list to show the full list. [Testing completed]: Yes [Risk to taking this patch] (and alternatives if risky): Low [String changes made]: No Since this patch is uplifted from v1.3t(with one line conflict in test), I didn't request the review for it.

Comment on attachment 8446341 [details] Link to github NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): Regression from message draft feature. [User impact] if declined: User could not drag down the recipient list to show the full list. [Testing completed]: Yes [Risk to taking this patch] (and alternatives if risky): Low [String changes made]: No

Comment on attachment 8448604 [details] Link to github(for v1.4) Adding verifyme for the patch to be verified once it lands on 1.4 and 2.0

v2.0: https://github.com/mozilla-b2g/gaia/commit/2dfe43bdeeb09aecfccc8bb1cc27dcf8bd315d98 v1.4: https://github.com/mozilla-b2g/gaia/commit/cb1ad02ba2dc1f0a912153d660209dfad9d3bce4

Issue is verified fixed in Flame 2.2, 2.1, 1.4 (Full Flash, nightly) Actual Results: "To:" field is scrollable when multiple contacts are selected. Device: Flame Master Build ID: 20141024040202 Gaia: d893a9b971a0f3ee48e5a57dca516837d92cf52b Gecko: a5ee2769eb27 Version: 36.0a1 (Master) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Device: Flame 2.1 Build ID: 20141024001204 Gaia: 0f76e0baac733cca56d0140e954c5f446ebc061f Gecko: 7d78ff7d25b6 Version: 34.0 (2.1) Firmware Version: v188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 1.4 Build ID: 20140814202332 Gaia: 129211661489feb60bbd6772a44081d23b374f17 Gecko: 1483bd406aad0b3b9e3bdb75c5238b787b5a0cd6 Version: 30.0 (1.4) Firmware Version: v165 User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Issue is verified fixed in 1.3t Tarako build. Actual Results: "To:" field is scrollable when multiple contacts are selected. Device: Tarako Build:20140708014001 Base: SP6821a-Gonk-4.0-5-12