958245 - [APZ][email] email compose content iframe no longer scrolls

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. turn apz on
2. launch email
3. create a new email
4. tap in the content and type enough to cause scrolling in the iframe
5. tap the subject line
6. tap the power button to turn off screen
7. tap the power button to turn on screen
8 unlock the phone
9. try to scroll the content

Expected: you can scroll the message content
Actual: you cannot scroll the message content

Note:
1. does not occur with APZ turned off

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Steps 6-8 are not needed in the STR. This just looks like we're not layerizing the input area for message content.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

This is a gaia bug; they are setting overflow:hidden on textareas. Presumably this was done to hide the scrollbar, but it also has the effect of making the textarea unscrollable (with APZ enabled).

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch to enable scrolling in email composer

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8358572 [details] [diff] [review]
Patch to enable scrolling in email composer

I don't know if you guys want to blanket remove these things or do something else.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8358572 [details] [diff] [review]
Patch to enable scrolling in email composer

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

Let's ask that to some email folk.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8358572 [details] [diff] [review]
Patch to enable scrolling in email composer

These both seem reasonable to me.  Specifically:

- shared/style/input_areas.css: This makes sense to me; all input areas should be scrollable.  I assume this was for visual reasons.  I certainly don't see any alternatives to removing the line, but I don't have review authority for the building blocks.  I guess Vivien does, but I'll r/f? both the people listed as peers specifically covering the building blocks at https://wiki.mozilla.org/Modules/FirefoxOS#Gaia.

- apps/email/style/compose_cards.css: Absolutely fine.  I'm sure this was for visual reasons from long ago, and possibly just cribbed from the above file.  We absolutely want to allow long subjects that require scrolling. r=asuth for this change in e-mail.


Note: When attaching diffs/patches, please provide 8 lines of context.  https://developer.mozilla.org/en-US/docs/Mercurial_FAQ has good tips on how to do that.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Andrew Sutherland (:asuth) from comment #6)
> Note: When attaching diffs/patches, please provide 8 lines of context. 
> https://developer.mozilla.org/en-US/docs/Mercurial_FAQ has good tips on how
> to do that.

Yeah, I didn't bother generating a proper patch since I wasn't sure if this approach was good to take or not. I can do that and submit a pull request.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Enable scrolling in all textareas and the email composer

Updated with commit message and proper diff.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

(In reply to Andrew Sutherland (:asuth) from comment #6)
> - apps/email/style/compose_cards.css: Absolutely fine.  I'm sure this was
> for visual reasons from long ago, and possibly just cribbed from the above
> file.  We absolutely want to allow long subjects that require scrolling.
> r=asuth for this change in e-mail.

Just wanted to clarify: this is for scrolling the email body compose area, not the subject line.

Comment 10

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #9)
> Just wanted to clarify: this is for scrolling the email body compose area,
> not the subject line.

Ah, the diff's context-providing logic got confused and then so did I.  The rationale holds for all cases though.  Scrollbars for all!

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8359242 [details] [diff] [review]
Enable scrolling in all textareas and the email composer

Then I think you want a review from Andrew not Kaze :)

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8359242 [details] [diff] [review]
Enable scrolling in all textareas and the email composer

to be clear, I think 2 reviews are needed here.  One for email (given, r=asuth), one for the building blocks change.  The building blocks reviewers were listed as :kaze and :basiclines, which is why I added both of them (though only one as a reviewer because I ran out of combo-boxes in the UI).  I'll just leave it :basiclines for now, but technically :kaze can review too?

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8359242 [details] [diff] [review]
Enable scrolling in all textareas and the email composer

Ismael has left so it may take a while for it to review that. I'm pretty sure Arnau is enough since he has probably wrote that with Ismael.

Comment 14

[Arnau March [:arnau] ( not working in Firefox OS anymore :( )]

Comment on attachment 8359242 [details] [diff] [review]
Enable scrolling in all textareas and the email composer

Looks good to me :)

Comment 15

[Kartikaya Gupta (email:[e-mail])]

Pull request at https://github.com/mozilla-b2g/gaia/pull/15363

Comment 16

[Kevin Grandon :kgrandon]

Landed: https://github.com/mozilla-b2g/gaia/commit/07f64fff89ea2dc39af1438eb40fba153b0f4d1c

Comment 17

[Kartikaya Gupta (email:[e-mail])]

We'll probably want this uplifted to 1.3 as well.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

Requesting uplift to 1.3.

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 07f64fff89ea2dc39af1438eb40fba153b0f4d1c to:
v1.3: 0732eacbccde029402aec98c3587d8cda764dde8

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Thanks

Comment 21

[Ross Kuhlman ([e-mail])]

This issue is fixed in 1.3 and 1.4. The user can scroll through the email body while the cursor is located in the subject field.

Environmental Variables:
Device: Buri v1.4 Moz RIL
BuildID: 20140123040608
Gaia: 00d8d05f0d0730a3cbf17635ad6a6b197a2ce7c9
Gecko: 1a75d37e1e39
Version: 29.0a1
Firmware Version: v1.2-device.cfg


Environmental Variables:
Device: Buri v1.3 Moz RIL
BuildID: 20140123004001
Gaia: 6fbeac2415f07f10de181f0877ddf67ee299b885
Gecko: e8f6bdf8db3d
Version: 28.0a2
Firmware Version: v1.2-device.cfg

Comment 22

[Andrew Stole [:astole]]

Since this has already been verified fixed in both 1.3 and 1.4, removing the verifyme tag.