Gaia/SMS/Scrum/2.1S5

From MozillaWiki
Jump to navigation Jump to search

List of bugs

SMS issues handled by the SMS subteam (blocks the sprint bug 1067820)

Bugzilla link

Full Query
ID Assigned to Summary Blocking b2g Feature-b2g Whiteboard Resolution
903763 [Messages] avoid css recalculations while rendering a thread --- No cf_feature-b2g [c=progress p= s= u=][p(2.1S5)=1] DUPLICATE
1003843 Oleg Zasypkin [:azasypkin] Follow up to Bug 951676 - [Messages][Refresh] Add specific image in case of more than one recipient in the list threads --- No cf_feature-b2g groupmms [p(2.1S6)=1][p(2.1S5)=1] FIXED
1021608 Steve Chung [:steveck] [Messages] Report panel visual refresh --- No cf_feature-b2g [sms-most-wanted][p(2.1S7)=1][p(2.1S6)=2][p(2.1S5)=2] FIXED
1053708 Julien Wajsberg [:julienw] Make SMS main User Interface RTL-friendly --- No cf_feature-b2g [p(2.1S6)=1][p(2.1S5)=2] FIXED
1054989 Julien Wajsberg [:julienw] [Messages][DSDS] Send button refresh --- No cf_feature-b2g [p=1] FIXED
1061417 Julien Wajsberg [:julienw] [SMS] We don't properly discard the draft in some occasions 2.1+ No cf_feature-b2g [g+][LibGLA,TD92485,QE1, B][p=1] FIXED
1067228 Oleg Zasypkin [:azasypkin] [Messages][Refactoring] Move subject management to a separate component --- No cf_feature-b2g [p=2] FIXED

7 Total; 7 Open (100%); 0 Resolved (0%); 0 Verified (0%);


Remaining points and burndown chart

google chart api url for Sprint 2.1S5

Burndown chart
Remaining points
Start 10
Day 2 10
Day 3 10
Day 4 9
Day 5 9
Day 6 9
Day 7
Day 8
Day 9
Day 10
End


SMS issues handled by the SMS subteam outside of the sprint (contains whiteboard "sms-sprint-2.1S5")

Full Query
ID Assigned to Summary Blocking b2g Feature-b2g Whiteboard Resolution
1052447 Julien Wajsberg [:julienw] [Messages] when we're in an activity and we delete all messages in the thread, we should close the activity 2.0+ No cf_feature-b2g [g+][LibGLA, Dev, B][sms-papercuts][sms-sprint-2.1S5] FIXED
1061215 Oleg Zasypkin [:azasypkin] [Messages][Refactoring] Improve handling of remaining chars counter --- No cf_feature-b2g [sms-papercuts][sms-sprint-2.1S5] FIXED
1070890 Oleg Zasypkin [:azasypkin] [Messages][Refactoring] Make use of EventDispatcher in Compose --- No cf_feature-b2g [sms-sprint-2.1S5] FIXED

3 Total; 3 Open (100%); 0 Resolved (0%); 0 Verified (0%);


All SMS issues tracked for this sprint (target milestone)

Bugzilla link

Full Query
ID Assigned to Summary Blocking b2g Feature b2g Resolution
836690 Jorge Prudencio [:jorgep] [SMS] Implement temporary in App message informing the user that they have started another SMS packet --- --- FIXED
1052447 Julien Wajsberg [:julienw] [Messages] when we're in an activity and we delete all messages in the thread, we should close the activity 2.0+ --- FIXED
1054989 Julien Wajsberg [:julienw] [Messages][DSDS] Send button refresh --- 2.2+ FIXED
1061417 Julien Wajsberg [:julienw] [SMS] We don't properly discard the draft in some occasions 2.1+ --- FIXED
1067228 Oleg Zasypkin [:azasypkin] [Messages][Refactoring] Move subject management to a separate component --- --- FIXED
1067820 [meta] SMS subteam sprint 2.1S5 --- --- WONTFIX

6 Total; 6 Open (100%); 0 Resolved (0%); 0 Verified (0%);


Sprint planning

Minutes are on a separate page.

Daily meetings

Day 2: 17th September

Julien

  • I finished reading my mails and handling some bugs
  • created the Scrum page; I updated the Template page (the graph URL was incorrect when the points were not 11). Maybe we could easily convert this to a script instead, now.

Today:

Oleg

  • bug 1067228 - [Messages][Refactoring] Move subject management to a separate component
    • Almost finished with the main parts, today will tune and work on tests (in progress);
    • Just realized that this bug depends on "bug 1061215 - [Messages][Refactoring] Improve handling of remaining chars counter" that is currently in r? for Steve. Will rebase on master and redirect to Julien as Steve is on PTO.

Other:

  • Investigated and dig through bugzilla about iframe vs unload/beforeunload. Alive asked to redirect that question to platform as it's Gecko who doesn't support that.
    (Julien) did we file a bug for having a "onclose" event for a MessagePort ?
    (Oleg) No, does spec contain that event?
    (Julien) nothing in the spec, but from what we saw we need it... we need to evolve the spec :)
    (Oleg) Ok, let's wait for Andrea reply first as start point, he is working on MessagePort and BroadcastChannel (MessageChannel too :)).
    (Julien) if we don't have an answer today, maybe send a mail to dev-webapi. Andrea can be busy :)
    (Oleg) Okay, thanks for dev-webapi reference :) I need to subscribe :)
  • Added few demos to previous sprint page.

Today:

  • Will handle review comments for the patches that currently in review;

Steve

  • Absent/no report

Luke

  • Absent/no report

Day 3: 18th September

Julien

  • bug 1061417 (draft is not discarded in some occasions): it was easier than expected, and a long-standing bug probably existing since v1.4 (in review)
  • resumed working on bug 1054989 (DSDS send button refresh), should have a patch ready today
  • did some reviews
  • looked into the strange blob-related issue that appeared while I was in holidays (bug 1059233); should have been fixed in gecko... (bug 1063658 might be this)

Other:

  • resumed work on bug 874510 (upgrade mocha); one piece was missing (bug 1035133) that I added and is in review now.

Today:

  • will continue working on bug 1054989
  • will dig in some issues I left from yesterday

Oleg

  • bug 1067228 - [Messages][Refactoring] Move subject management to a separate component
    • Still in progress (in progress);
  • bug 1061215 - [Messages][Refactoring] Improve handling of remaining chars counter
    • Got first review feedback, working on it (in progress).
      (Julien) do you need something from me here? I saw a question but I don't know if you still need the answer :)
      (Oleg) I guess no :) Will ping you on IRC if have one :)
      (Julien) ok great !
  • bug 1067712 - [MADAI] [SMS] Message Apps cannot remove surrogate pair character correctly
    • Investigated and provided reduced test case, now we have Core->Editor bug instead
      (Julien) I hope we don't need an uplift :/
      (Oleg) Yeah, I believe it was always there

Today:

  • Will handle review comments for the patches that currently in review;
  • Will handle review requests.
  • Will see what events are fired on iframe in Firefox when it's removed from DOM to be sure that's not only gaia problem.

Steve

  • Absent/no report

Luke

  • Absent/no report

Day 4: 19th September

Steve

  • bug 1058459 - [SMS] Data shared from other applications are not shown as Drafts until the app is killed and open again
    • Reviewing the SharedWorker patch(and it's brilliant!) will leave some comments later and give it a try
  • bug 1059233 - [SMS] User cannot view the cropped image in SMS application and ActivityCanceled error was shown when try to view
    • Having a integration tests for this case might be an idea to avoid possible regression(removing the workaround without gecko fixed first) like this. I've checked 1.4 and this issue exist since workaround was removed from v1.4 and later.
      (Julien) yeah it's strange that the branch checks said that it works in v1.4. I'll look closer today...
  • bug 1067232 - [SMS] Avoid to make a private memory copy for file based blob
    • No update yet.
      (Julien) please wait that I sort out the issue in the platform; I think I'll just back out the workaround in SMS and we'll either add it back in Gallery or find the fix in the platform. :asuth thinks that even with the current workaround we can fail in some cases.
      (Steve) Hmm, not sure which case will make the blob copy workaround failed, I thought both workaround have similar approach.
      (Julien) not sure either. Nope, the gallery workaround is: copy the blob to a temporary file so that we get a file-backed blob and do not have the issue in SMS.
      (Steve) Ah, I see. Just check the gallery's patch.
      (Julien) I haven't looked at the patch though, I may be wrong here, I just take what djf told me :) Anyway we'll know more today !
      (Steve) I could agree it's better to workaround in gallery then in messaging in general like asuth said, if they(gallery) agrees to do so. I just confused that why we need messaging workaround in 1.2f and 1.3t if gallery already got the workaround as well...
      (Julien) the gallery workaround is in v1.3t only, as far as I know.
  • bug 1021608 - [Messages] Consider adding a "resend" button in the message report page, if there is an error, and other visual refresh
    • Start to implement report panel refresh.
  • bug 1068564 - When CMAS alert are deleted in notifications panel, device closes utility try
    • Checking with current master and other notification case. It's weird and seems not a CMAS app wise issue.
      (Oleg) It looks like what I saw previously https://bugzilla.mozilla.org/show_bug.cgi?id=1051793#c8
      (Steve) Thanks! I almost forgot this. It was worked on emulator, but I haven't revisit again
      (Oleg) Maybe I'm wrong but as I remember when I swiped notification it got activated and our app reacted... don't remember exactly :)
      (Julien) it's probably a system front-end bug. (but probably from my code from bug 1051788 :D)
      (Steve) Hmm, will take a look about this then, thanks too!

Today:

Julien

  • bug 1061417 (draft is not discarded in some occasions): landed
  • bug 1054989 (DSDS send button refresh):
    • got a patch, waiting for fang's ui-review and maybe image updates (because the new ones are larger for apparently no reason)
    • also added a missing l10n string
    • might ask for an approval to land on v2.1 if not too late
  • did some reviews for Oleg especially :)
  • looked into the strange blob-related issue that appeared while I was in holidays (bug 1059233); should have been fixed in gecko... (bug 1063658 might be this)
    • looks like there is something deeper than just SMS. Will look further today, especially see how it works in the various versions. The regression window just points to the workaround removal in Gallery which does _and_ doesn't make sense ;)

Other:

  • resumed work on bug 874510 (upgrade mocha) (my "subway" project :) ); fixed some unit tests but there are more failing.

Today:

  • will continue working on bug 1059233 for the blob related issue
  • will do reviews
  • will start another sprint bug (don't know which one yet)

Oleg

  • bug 1067228 - [Messages][Refactoring] Move subject management to a separate component
    • Almost finished unit tests, will wait for bug 1061215 first(in progress, almost finished);
  • bug 1061215 - [Messages][Refactoring] Improve handling of remaining chars counter
    • Got second round of review comments, fixed, will ask review in a few! (in progress).
  • bug 1050416 - [Messages][Build] Add build script to ignore the desktop-only folder for production build
    • Got first round of review comments, working on it (in progress).

Other:

  • Reviewed Luke's patch to fix cursor glitch, left some suggestion on selection managment and unit test;
  • Reviewed small patch on draft discard.

Today:

  • Will handle review comments for the patches that currently in review;
  • Will handle review requests.

Day 5: 22th September

Steve

  • bug 1021608 - [Messages] Consider adding a "resend" button in the message report page, if there is an error, and other visual refresh
    • Ongoing.
  • bug 1068564 - When CMAS alert are deleted in notifications panel, device closes utility try
    • I could only reproduce this on CMAS, and like Oleg said it's not just on clear all notification case, every CMAS notification removed by swipe will also hide the tray. So I will take a deep look now. Not quite sure that it's about the silent notification changes in bug 1026685. The only thing I know now is the hide action occurred after mozContentNotificationEvent dispatched.
      (Julien) tell me if you'd prefer to redirect to me :)
      (Steve) If I can't find answer today, it might be more efficient that hand over this bug to you
      (Julien) ok, tell me when you go home later today :)
  • Some review and bug reply.

Today:

Julien

  • bug 1054989 (DSDS send button refresh):
    • still in review
      (Oleg) will take a look very soon, sorry :)
  • looked into the strange blob-related issue that appeared while I was in holidays (bug 1059233); should have been fixed in gecko... (bug 1063658 might be this)
    • spent a lot of time on this. Found a similar issue without cropping, but when we use an image that needs an EXIF-based rotation; this also triggers a memory blob. I don't understand why the use case with crop works in v1.4 but not the use case with Exif.
    • my preference would be to reland the workaround in gallery instead of SMS, because other applications (eg: email) have the issue
    • I'd also like to understand why the crop-based use case work in v1.4. I think I'll look at it again today.
      (Steve) Did you tried it yourself? Not sure how they test this issue, but it's only reproducible on image smaller than size limitation.
      (Julien) Yep I tried myself :) (I have 1 phone for each version...). but you're right, I'll check if I used an image that was small enough.
      (Steve) BTW I used 9/11 1.4 build on bury
  • starting looking at bug 1003843 (change how we display threads with several recipients)
    • asked a question to Fang about this

Other:

  • resumed work on bug 874510 (upgrade mocha) (my "subway" project :) ); fixed some unit tests but there are more failing.

Today:

  • will continue working on bug 1063658 for the blob related issue
  • will do reviews
  • will work on bug 1003843 if Fang is answering, otherwise I'll take either the RTL bug or bug 903763 for performance.

Oleg

  • bug 1067228 - [Messages][Refactoring] Move subject management to a separate component
    • Done, waiting for dependencies to land (waiting for dependency);
  • bug 1061215 - [Messages][Refactoring] Improve handling of remaining chars counter
    • Fixed the latest comments (in review);
    • Added big marionette test to test MMS label and char counter behaviour :)
    • Rebased on the latest master as patch for bug 836690 resurrected ThreadUI<->Char counter dependency, not sure how segment info notice should really work, asked Steve on github.
      (Julien) doesn't ThreadUI just use the "segmentinfo" event? (I haven't looked the new code)
      (Oleg) Yes, it uses it, but also uses counterLabel.dataset.counter to get previous segment info (parses and splits content to get segment number). I was thinking about passing old and new segment info in segmentinfochange event
      (Julien) in DOM, usually, we don't pass the information with the event if it can be retrieved in another way (eg: geolocation vs visibilitychange). Maybe ThreadUI could just store the segmentinfo itself. But I don't really mind :)
      (Oleg) Yeah, that is what I did currently, but I'm storing only segment number for now
      (Julien) yep, I meant segment number.
  • bug 1050416 - [Messages][Build] Add build script to ignore the desktop-only folder for production build
    • Created asyncStorage mock that doesn't touch IndexDB, so it keeps drafts in memory + other fixes; Didn't touch regex part for now (in review).

bug 1014686 - [Messages][Refactoring] Improve "html template string" to "dom node\fragment" conversion

    • Provided patch for Template.prepare().toDocumentFragment as I want to use something like this in subject refactoring patch (in review). Tried to check its performance on JSPerf - looks fine to me, if I did everything correctly :) (in review)

Today:

  • Will handle review comments for the patches that currently in review;
  • Will handle review requests.

Day 6: 23th September

Steve

  • bug 1021608 - [Messages] Consider adding a "resend" button in the message report page, if there is an error, and other visual refresh
    • Ongoing.
  • bug 1068564 - When CMAS alert are deleted in notifications panel, device closes utility try
    • After some investigation, it should be the notification issue and it seem exist long time ago. Notification will dispatch the close system message, and it will trigger the app launch again(and app launch event will hide the tray). It seem not a reasonable patent anyway, so I redirect the issue to system platform side.
  • bug 1068490 - [Wooduck][[Comms]MMS]The picture will flicker once when you take one picture as attachment
    • Currently we always append the image attachment node at first, and update the attachment node if the total size exceed the limitation. This node replacement action might not the necessary step. Maybe we could simply update the size indicator and attachment item.
      (Julien) another idea is to put the default image for <image> attachments while we resize, and add the resized attachment on top (so that it won't flicker). I'll comment a little more on the bug :)
  • Some review and bug reply.
  • Some copy paste issue in message, but George Duan will handle it first. So if you see any problem related to copy paste issue, please refer him or Morris Tseng (gecko) for more information.
    (Julien) Good to know, thanks !

Today:

Julien

  • bug 1054989 (DSDS send button refresh):
    • still waiting for Fang's ui-review :(
  • looked into the strange blob-related issue that appeared while I was in holidays (bug 1059233); should have been fixed in gecko... (bug 1063658 might be this)
    • haven't looked more yesterday, need to look again...
  • about bug 1003843 (change how we display threads with several recipients)
    • unassigned myself to take another bug
    • Fang answered today so it's ready if one of you wants to take it
  • started bug 1053708 (make main interface RTL friendly)
    • opened some bugs in Gecko to make this easier; will open more today and also send mails to www-style. Some CSS properties are ready for RTL, some others are not, I'd like to at least move forward this matter at W3C. In the mean time we'll probably need to override some CSS, but at least this will make the web better over time :)

Other:

  • we have a new 2.0 blocker (bug 1052447). Should be an easy one. Who can take it?
    (Julien) I can do it if you're busy on other things :)
    (Oleg) I'll check, haven't noticed it yet :)

Today:

  • will continue working on bug 1063658 for the blob related issue
  • will do reviews
  • will continue work on bug 1053708

Oleg

  • bug 1067228 - [Messages][Refactoring] Move subject management to a separate component
    • Done, waiting for dependencies to land (waiting for dependency);
  • bug 1061215 - [Messages][Refactoring] Improve handling of remaining chars counter
    • Got next review round comments, asked clarifying questions (waiting for reply).
      (Julien) ok, will look at it first
      (Oleg) thanks!
      (Julien) may be easier/faster to wrap this up on IRC :)
      (Oleg) Yep :)
      (Julien) ok let's do this after the meeting
  • bug 1058459 - [SMS] Data shared from other applications are not shown as Drafts until the app is killed and open again
    • Dig into possibility to MessagePort.onclose - looks like no way - "bug 915880 - Add onstart/onclose event handlers in the MozInterAppMessagePort" and "http://lists.whatwg.org/pipermail/whatwg-whatwg.org/2013-October/252687.html". There is a big discussion that onclose would expose GC behaviour that isn't good.
    • So as per :baku we have weak reference from port to owner (window that owns port) and having dead ports we leak only these ports, well it's bad, but not as bad if we leaked entire window reference :) Probably we can do the following:
      • On "unload" - owner post message to SharedWorker like "I'm dying" and closes the port as recommended by spec; Shared worker will get this message and removes reference to the port. This won't work for the case when app is crashed or killed by LMK.
        (Julien) IIRC we don't always get "unload"? Also I don't know if we have access to some API in "unload".
        (Oleg) Yeah app can "miss" unload, so that we need fallback cleanup plan :( I've checked - postMessage works from unload handler.
        (Oleg) But I'm not sure what happens if app is killed and didn't have chance to close port...
      • For the later case SharedWorker can track number of active connections and after eg. 5 it can ping all it's ports to check if they alive - thus we'll remove overhead by pinging\polling constantly and will ping only in case something went wrong...
    • What do you guys think?
      (Julien) what happens if we don't know that a port is dead? Is it really an issue?
      (Oleg) I believe 5 or even 10 dead ports won't be a problem - it's just reference to port object, probably it depends on usage pattern - if user never closes main app and sharing-sharing-sharing :)
      (Julien) I don't find any better solution than this... I'm kind of sad ;)
      (Oleg) Yeah, I don't like hacks :( Seems BroadcastChannel API will also have this problem. Not sure whether we should move forward with it
      (Julien) But for BroadcastChannel we don't need to manage ports directly, right? It's all hidden in Gecko?
      (Oleg) Mmm, I need to see spec, probably I'm wrong :)
      (Oleg) Yeah, it's better but it still has "close" method that app should call at some point. So here we only need to take care about app crash case somehow
      (Julien) but here Gecko will take care of the GC-ed port because it's all internal so I don't think we'd have to worry. But anyway it's not ready so let's not lose time about BroadcastChannel now :)
      (Oleg) Yep, so should we move forward with SharedWorker or wait until it becomes a blocker for someone?
  • bug 1070890 - [Messages][Refactoring] Make use of EventDispatcher in Compose
    • Extracted this part from Subject refactoring patch to make review easier (in review).
      (Steve) Reviewed!
      (Oleg) Ah, cool, thanks!

Today:

  • Will handle review comments for the patches that currently in review;
  • Will handle review requests.

Day 7: 24th September

Day 8: 25th September

Day 9: 26th September

Day 10: 29th September

Demos

Retrospective