Closed
Bug 1190275
Opened 9 years ago
Closed 9 years ago
[Messages] Image attachments are not showing up properly when going to a subview and coming back
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: julienw, Assigned: julienw)
Details
(Whiteboard: [sms-sprint-FxOS-S4])
Attachments
(1 file)
STR:
1. Go to a thread that has MMS with image attachments that are not displayed initially (more than 10 messages happened after this one).
2. Display the report view for one of the displayed messages.
3. Press the cross to come back to the conversation.
4. Scroll up to the aforementioned message.
Expected:
* We see the image attachment.
Actual:
* There is a grey square instead of the image.
Comment 1•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Whiteboard: [sms-FxOS-S4]
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8642346 [details]
[gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview > mozilla-b2g:master
Hey Oleg,
what do you think of this simple change ?
I can't think of an integration test for this, unless we can take screenshots maybe ?
Attachment #8642346 -
Flags: review?(azasypkin)
Assignee | ||
Updated•9 years ago
|
Whiteboard: [sms-FxOS-S4] → [sms-sprint-FxOS-S4]
Comment 3•9 years ago
|
||
Comment on attachment 8642346 [details]
[gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview > mozilla-b2g:master
(In reply to Julien Wajsberg [:julienw] from comment #2)
> Comment on attachment 8642346 [details]
> [gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview >
> mozilla-b2g:master
>
> Hey Oleg,
>
> what do you think of this simple change ?
Looks good, but I'm wondering if we leak memory when we navigate from Conversation to view other than Inbox, left question at github.
>
> I can't think of an integration test for this, unless we can take
> screenshots maybe ?
Yeah, I think screenshot can do the job, smth like:
client.screenshot({
element: elementToScreenshot
});
But if it's turned out to be complex, I'm fine to not do it in this patch.
Thanks!
Attachment #8642346 -
Flags: review?(azasypkin)
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8642346 [details]
[gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview > mozilla-b2g:master
Hey Oleg,
what do you think ?
I merged everything in one commit because the rebase I did made the additional changes not really obvious...
Attachment #8642346 -
Flags: review?(azasypkin)
Comment 5•9 years ago
|
||
PR looks good to me, I've left just few questions/nits for the integration test part, so keeping r? flag for now.
Comment 6•9 years ago
|
||
Comment on attachment 8642346 [details]
[gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview > mozilla-b2g:master
[review queue cleanup]: please set r? again whenever you're ready :)
Attachment #8642346 -
Flags: review?(azasypkin)
Assignee | ||
Comment 7•9 years ago
|
||
Oops I totally missed you answered me with my old bugmail...
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8642346 [details]
[gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview > mozilla-b2g:master
Ready for a second review round :)
You can look at the 3rd commit in [1] for my try using scrollUp instead of scrollUpTo but it was really difficult to get right. Even if I manage to get it right I'm very afraid of intermittents :/
[1] https://github.com/mozilla-b2g/gaia/commit/6a10ba6479bb5bc9919f571a2ab4e6de6daf994e
Attachment #8642346 -
Flags: review?(azasypkin)
Comment 9•9 years ago
|
||
Comment on attachment 8642346 [details]
[gaia] julienw:1190275-dont-revoke-blob-urls-when-going-to-subview > mozilla-b2g:master
r=me, just fix one integration test related nit.
(In reply to Julien Wajsberg [:julienw] from comment #8)
> You can look at the 3rd commit in [1] for my try using scrollUp instead of
> scrollUpTo but it was really difficult to get right. Even if I manage to get
> it right I'm very afraid of intermittents :/
Yeah, let's leave it for the future, maybe we'll come up with something simpler.
Thanks!
Attachment #8642346 -
Flags: review?(azasypkin) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•