1154156 - [Message] The image attachment can't be compressed sometimes.

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Wei Gao (Spreadtrum)]

OS version
---------------------------------------------
FireFoxOS v2.1

I found log showed that:

01-28 14:13:51.255  1801  1801 E GeckoConsole: [JavaScript Error: "TypeError: target.getBoundingClientRect is not a function" {file: "app://sms.gaiamobile.org/js/compose.js" line: 565}]

It seems the JavaScript Error is the cause.
But I can't find the reason.

        if (document.activeElement === dom.message) {
          // insert element at caret position
          var range = window.getSelection().getRangeAt(0);
          var firstNodes = fragment.firstChild;
          range.deleteContents();
          range.insertNode(fragment);
          this.scrollToTarget(range);
          dom.message.focus();
          range.setStartAfter(firstNodes);
        } else {
          // insert element at the end of the Compose area
          dom.message.insertBefore(fragment, dom.message.lastChild);
          this.scrollToTarget(dom.message.lastChild);
        }
        if (!options.ignoreChange) {
          onContentChanged(item);
        }

Comment 1

[Wei Gao (Spreadtrum)]

The JavaScript Error occured at:

https://github.com/mozilla-b2g/gaia/blob/v2.1/apps/sms/js/compose.js#L565

Dear Julien

Do you have any idea of it.
Thanks.

Comment 2

[Julien Wajsberg [:julienw]]

If you have a STR it would be nice.

Otherwise you can try adding logs to the scrollToTarget function:

  console.log("scrollToTarget", target, new Error().stack.replace(/\n/g, '|'));

So that when this happens we know exactly what is the target (especially if it's null ?) and what is the stack trace.

Comment 3

[Wei Gao (Spreadtrum)]

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #2)
> If you have a STR it would be nice.
> 
> Otherwise you can try adding logs to the scrollToTarget function:
> 
>   console.log("scrollToTarget", target, new Error().stack.replace(/\n/g,
> '|'));
> 
> So that when this happens we know exactly what is the target (especially if
> it's null ?) and what is the stack trace.

I don't know the STR either, our QA said that he added attachments normally and didn't do any special operation.
I will add your log into our branch to reproduce it.
Thanks for your attention.

BTW, the following if() statement seems that could not be executed, because when I click textbox, the document.activeElement is dom.message, then I click attachment button to add attachment, the document.activeElement will become attachment button, so I can't insert image to where I want.

if (document.activeElement === dom.message) {
          // insert element at caret position
          var range = window.getSelection().getRangeAt(0);
          var firstNodes = fragment.firstChild;
          range.deleteContents();
          range.insertNode(fragment);
          this.scrollToTarget(range);
          dom.message.focus();
          range.setStartAfter(firstNodes);
        }

Comment 4

[Julien Wajsberg [:julienw]]

You're totally right, I filed bug 1154170 for this.

Comment 5

[Julien Wajsberg [:julienw]]

Can you confirm if in your code line 565 is:

      var targetRect = target.getBoundingClientRect();

or

      var containerRect = dom.message.getBoundingClientRect();

I think it's the first one but let's be sure :)

Comment 6

[Wei Gao (Spreadtrum)]

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #5)
> Can you confirm if in your code line 565 is:
> 
>       var targetRect = target.getBoundingClientRect();
> 
> or
> 
>       var containerRect = dom.message.getBoundingClientRect();
> 
> I think it's the first one but let's be sure :)

Yes, I am sure about it.
I have checked it again, in our local branch, compose.js is not modified by anyone.
Thanks.

Comment 7

[Wei Gao (Spreadtrum)]

line 565 is:

var targetRect = target.getBoundingClientRect();

Comment 8

[Julien Wajsberg [:julienw]]

What could happen is that, in "append", we call:

  this.scrollToTarget(dom.message.lastChild);


and if lastChild is a Text Node (as opposed to an Element), then it does not have getBoundingClientRect.

When we designed this code, lastChild was always a <br> element and we were taking care of this. But maybe some of the code has changed.


Another thing you can log in scrollToTarget is "dom.message.innerHTML".
You can maybe put the "scrollToTarget" call between a try/catch block, so that you can log things only when there is an exception:

  try {
    this.scrollToTarget(dom.message.lastChild);
  } catch(e) {
    console.log(...); // log
    throw e; // rethrow the exception
  }

Comment 9

[Wei Gao (Spreadtrum)]

(In reply to Julien Wajsberg [:julienw] (PTO April 6th) from comment #8)
> Another thing you can log in scrollToTarget is "dom.message.innerHTML".
> You can maybe put the "scrollToTarget" call between a try/catch block, so
> that you can log things only when there is an exception:
> 
>   try {
>     this.scrollToTarget(dom.message.lastChild);
>   } catch(e) {
>     console.log(...); // log
>     throw e; // rethrow the exception
>   }

Yes, I certainly agree with it. This can avoid large numbers of useless log.
Thanks for your suggestion.

Comment 10

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 11

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(