Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 961637 - [email/UI] Quick advance regressed envelope display #15745

Merged

Conversation

jrburke
Copy link
Contributor

@jrburke jrburke commented Jan 27, 2014

The fix was two-fold:

  • clearDom was a bit aggressive with the innerHTML clearing, just the peep bubbles needed to be removed, and make sure addHeaderEmails removes any previously collapsed state on a peep section if it means for it to be displayed.
  • postInsert is no longer the right conceptual hook to use to display the message, onCurrentMessage is, so just moved the postInsert work into onCurrentMessage, as the other work done in onCurrentMessage needs to be done for each message displayed anyway. So, no more doubling of the peeps.

I did not attach a test as I feel we are still stabilizing the tests. I would prefer land a fix vs. creating more test code that may cause intermittents, and this is a very noticeable regression that impacts people dogfooding on master.


// if the body reps are downloaded show the message immediately.
if (body.bodyRepsDownloaded) {
this.buildBodyDom();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a comment here (or somewhere around here) that indicates that it's okay for us to be calling this because our call to getBody guarantees that we are being called AFTER the card has been initialized and spliced into the DOM so that our iframe creation will work correctly. postInsert originally existed for this purpose, but that also assumed we already had the body instance.

I worry that without this comment we/someone might shuffle this code a little more in the future, things will break for only HTML e-mails, and they'll be deeply confused what that's happening. Of course, I guess we could also just have our iframe insertion code check if the DOM tree it's working with is present in the tree and explicitly throw/do something in a way that breaks the unit tests and is very obvious from the logcat...

jrburke added a commit that referenced this pull request Jan 28, 2014
Bug 961637 - [email/UI] Quick advance regressed envelope display r=asuth
@jrburke jrburke merged commit 09ae464 into mozilla-b2g:master Jan 28, 2014
@jrburke jrburke deleted the bug961637-email-advance-envelope branch January 28, 2014 19:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants