Confirmed users
821
edits
Gaia/SMS/Scrum/2.2S5: Difference between revisions
→Day 3: 29th January
| Line 136: | Line 136: | ||
=== Day 3: 29th January === | === Day 3: 29th January === | ||
====Steve==== | |||
* Spending ALL DAY with partner about the partner requirement for 2.2(Partner WW in Taipei this week) Might not be able to join the stand up in time. | |||
* {{Bug|1089154}} - [Messages] investigate scoping CSS rules | |||
** Will provide a patch today with selector refinement. I tried to set display: none to the checkboxes, seems we could gain ~30ms from it. | |||
**: (Oleg) Yay! I remember we discussed something similar in the past, but probably forgot | |||
**: (Julien) initially we didn't want to set them to :none because we would lose the transition because of reflow... but now the transitions are starting after reflows, so maybe it's good. We need to check whether we have a bigger lag when enabling edit form. | |||
**: (Oleg) Anyway we can remove display:none after full or 1st page load | |||
**: (Julien) yeah, this too. | |||
* Request 2.2 approval for 2.1 blockers | |||
Today: | |||
* More testing replace selectors in action menu/edit mode. | |||
* code review | |||
====Julien==== | |||
* {{Bug|1089920}}: fix gaia-header performance | |||
** landed the gaia-header patch, today I'll do a PR for the SMS patch and submit it for review | |||
* {{Bug|1082618}}: add a README file | |||
** no progress | |||
* {{Bug|1122649}}: mozSettings has a synchronous penalty | |||
** {{Bug|1115796}}'s issue didn't move forward even if I gave Fabrice all proof | |||
* {{Bug|1087981}}: remove the load penalty of notify.js | |||
** implemented latest comments from Steve, asked feedback, will land today | |||
* did a lot of reviews for Rishav and Oleg; have some more to do today ;) | |||
Today: | |||
I want to (again): | |||
* more review | |||
* investigate if read-ahead value changes something in SMS | |||
* move forward some background matters (organize group MMS bugs, and drafts handling issues started some weeks ago) | |||
* land the README file | |||
====Oleg==== | |||
* {{Bug|1122501}} - [Messages][Drafts] Unsaved draft is silently discarded when user tries to forward message | |||
** Got question from Steve, reading and replying :) | |||
* {{Bug|1104961}} - Intermittent share_activity_test.js | Messages as share target Share via Messages Activity close button Should return to Thread panel if in Participants panel | |||
** Tried several approaches to tackle the problem with recipients.js: | |||
*** Was counting "recipientschange" event (1 event is fired when we send recipient number with marionette's sendKeys and 4 times when we assimilate recipient with Enter key), but actually found out that Marionette can sendKeys in a small batches so that number of "recipientschange" event isn't easily determined; | |||
*** The current solution is that I throttle Mutation events with 1sec timeout - I don't see intermittent failures in share_activity test anymore, but still not sure in the solution; | |||
*** Haven't tried yet solution with throttling of "recipientschange" event per Julien's suggestion. | |||
**:(Julien) Maybe the solution is to change how recipientschange is sent, after all :) doing a proper patch | |||
**:(Oleg) That's true, but it doesn't look like a simple and safe thing, and I want these test to be enabled as soon as possible because new patches are arriving all the time :) + we probably we'll have these intermittent failures in v2.2 branch | |||
**: (Julien) I don't really care about v2.2 as long as we have the test working in master. FWIW I'm fine with an ugly timeout for v2.2 :) Maybe an ugly timeout for master is also fine if we have a plan to remove it by refactoring the code. If the goal is to reenable the test quickly.. | |||
**: (Oleg) If you mean client.helper.wait - then it was my plan, but I see some resistance from Kevin :) | |||
**: (Steve) I'm fine this this mutation observer with timeout, but does this patch solve {{Bug|1121766}} as well? Because you also remove compose test from list | |||
**: (Oleg) 1121766 - nope, not entirely, I won't enable it in this patch. Yeah, as I replied on GitHub I just wanted to see if it helps as well - and it helps, but we have one more problem there - I'll address it in composer_test patch separately. | |||
**: (Oleg) So guys, what do you think is better for now? just client.wait or throttling with MutationEvent/"recipientschange"? | |||
**: (Julien) If Steve is ok with the mutation observer then i'm ok too; but it's also using a timeout so is it really better than client.wait? Kevin had resistance initially but if there is a plan to change it then I think he'd be ok. | |||
**: (Steve) Agree, we have refine the recipients.js for it in the future and make these test work first. | |||
**: (Oleg) Okay I'm leaving as is for now (MutationObserver + timeout, it still has better chances to work than plain timeout, unless timeout is ~3 sec :) ) and if Steve is not ok, then will change to just client.helper.wait. | |||
**:(Julien) good for me ! | |||
* {{Bug|1121766}} - Intermittent failing test, TEST-UNEXPECTED-FAIL | apps/sms/test/marionette/composer_test.js | Messages Composer Messages Composer Test Suite Message char counter and MMS label | |||
** The good news here is that I finally understand :) that there is only one (I hope) root cause for several different intermittent - "stale element" (recipient node is removed), "sendKeys timeout" (placeholder node is removed) and "email recipient can't be removed" - recipients.js that re-renders its content. | |||
* {{Bug|1124758}} - [Messages] We should not add the hidden (+0) text at the end of the header | |||
** Got first review comments, addressed and asked for review (in review) | |||
Today: | |||
* Will handle review/feedback/need-info requests; | |||
* Will work on review comments and assigned bugs | |||
=== Day 4: 30th January === | === Day 4: 30th January === | ||
=== Day 5: 2nd February === | === Day 5: 2nd February === | ||