Opening Troubleshooting info gives |TypeError: Argument 1 of Node.appendChild is not an object - chrome://messenger/content/about-support/init.js:106:5|

RESOLVED FIXED in Thunderbird 51.0

Status

Thunderbird
General
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

Trunk
Thunderbird 51.0

Thunderbird Tracking Flags

(thunderbird49 wontfix, thunderbird50 fixed, thunderbird51 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

9 months ago
+++ This bug was initially created as a clone of Bug #1281785 +++

This was observed testing bug 1281785, see bug 1281785 comment #12:

TypeError: Argument 1 of Node.appendChild is not an object.  init.js:106:5
appendChildren           chrome://messenger/content/about-support/init.js:106:5
populateGraphicsSection  chrome://messenger/content/about-support/gfx.js:250:5
window.onload            chrome://messenger/content/about-support/init.js:72:3
(Assignee)

Updated

9 months ago
Version: 48 Branch → Trunk
(Assignee)

Comment 1

9 months ago
Looks like bustage caused by:
http://hg.mozilla.org/mozilla-central/rev/533af0114689

We note that the table is built quite differently in M-C:
https://dxr.mozilla.org/comm-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/mozilla/toolkit/content/aboutSupport.js#293
(Assignee)

Comment 2

9 months ago
Created attachment 8791182 [details] [diff] [review]
WIP: v1.

Mason, with your change here:
http://hg.mozilla.org/mozilla-central/rev/533af0114689#l1.12
"incides" is now always present and that breaks our failure display here:
https://dxr.mozilla.org/comm-central/rev/c482d8a2cd6178a9e5990c07b6c7c09dc63d919d/mail/components/about-support/content/gfx.js#213

Dumping out data.indices.length I get 16, and without the hack of |if (false &&| trGraphicsFailures comes out as a string with ",,,,,,,,,,,,,,," which obviously cannot be appended as a node, so appendChildren(graphics_failures_tbody, trGraphicsFailures); fails.

Can you please advise on how to fix this. Do we just remove the if-part and leave the else-part?

I noticed that it's done differently but similarly here:
https://dxr.mozilla.org/comm-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/mozilla/toolkit/content/aboutSupport.js#293
Flags: needinfo?(mchang)
(Assignee)

Updated

9 months ago
Blocks: 1298085
(Assignee)

Comment 3

9 months ago
Mason, sorry about the noise. Our code was wrong from day one. Now that I get graphics failures, I noticed it. I have a fix already. Sorry again.
Flags: needinfo?(mchang)
(Assignee)

Comment 4

9 months ago
Created attachment 8791221 [details] [diff] [review]
Fix.

Here comes more traction ...
Assignee: nobody → jorgk
Attachment #8791182 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8791221 - Flags: review?(acelists)
(Assignee)

Comment 5

9 months ago
Created attachment 8791223 [details]
This is what I see with the patch applied and also the patch from bug 1302651.

This should settle the "Troubleshooting Information" saga.
(Assignee)

Comment 6

9 months ago
Note that the information given by FF is more sophisticated.

Compare FF:
https://dxr.mozilla.org/comm-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/mozilla/toolkit/content/aboutSupport.xhtml#371
        <tbody id="graphics-failures-tbody">
          <tr>
            <th colspan="2" class="title-column">
              &aboutSupport.graphicsFailureLogTitle;
            </th>
          </tr>
        </tbody>

with TB:
https://dxr.mozilla.org/comm-central/rev/c482d8a2cd6178a9e5990c07b6c7c09dc63d919d/mail/components/about-support/content/aboutSupport.xhtml#354
        <tbody id="graphics-failures-tbody">
        </tbody>

FF has nice headings, but that's beyond the scope of this bug.
(Assignee)

Comment 7

9 months ago
Created attachment 8791243 [details] [diff] [review]
Fix. (v2).

I guess I need the {failures: [], indices: []}; otherwise when there are no failures I'd get the "LogFailure" line since "indices" is not in data, right? I can't test that since I always get failures.

Can't you please test and then r+ the patch you want and obsolete the other. Thanks.
Attachment #8791243 - Flags: review?(acelists)
(Assignee)

Comment 8

9 months ago
Comment on attachment 8791221 [details] [diff] [review]
Fix.

This one doesn't work when you have no failures, since takes else here
if ("indices" in data && data.failures.length == data.indices.length) {
...
} else {
...
createParentElement("td", data.failures.map(

... and data.failures is not defined. So let's go with the other patch ;-)
Attachment #8791221 - Attachment is obsolete: true
Attachment #8791221 - Flags: review?(acelists)

Comment 9

9 months ago
Comment on attachment 8791243 [details] [diff] [review]
Fix. (v2).

Review of attachment 8791243 [details] [diff] [review]:
-----------------------------------------------------------------

I have no errors listed as you have in the screenshot. My last item in the Graphics block is "CairoUseXRender" (may be linux specific), after "AzureContentBackend".

The about:troubleshooting page looks fine now, everything populated till the end (Libraries).

Does that look correct?
Attachment #8791243 - Flags: review?(acelists) → review+
(Assignee)

Comment 10

9 months ago
You only ran into the problem when it tried to list the errors. I have errors and it works for me.

Landed:
https://hg.mozilla.org/comm-central/rev/4e6a8a13868c
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0
(Assignee)

Comment 11

9 months ago
Comment on attachment 8791243 [details] [diff] [review]
Fix. (v2).

This has been broken for a long time, so let's get it into Aurora for the next beta coming in a week.
Attachment #8791243 - Flags: approval-comm-beta?
Attachment #8791243 - Flags: approval-comm-aurora+
(Assignee)

Comment 12

9 months ago
Created attachment 8791343 [details] [diff] [review]
Follow up.

Aceman, sorry to trouble you again. I've thought about it, and I think we should align more with Firefox here:

https://dxr.mozilla.org/comm-central/rev/1851b78b5a9673ee422f189b92e5f1e86b82a01c/mozilla/toolkit/content/aboutSupport.js#293
    // graphics-failures-tbody tbody
    if ("failures" in data) {
      // If indices is there, it should be the same length as failures,
      // (see Troubleshoot.jsm) but we check anyway:
      if ("indices" in data && data.failures.length == data.indices.length) {

I think it's silly to pre-populate 'data', which is unnecessary if we do it the Firefox way. Sorry about the large patch, all it is is adding |if ("failures" in data) {}| and indenting.

I hope you, Mr. Perfection, agree.

This works for me when errors are present and it will work on your platform without errors.
Attachment #8791343 - Flags: review?(acelists)

Comment 13

9 months ago
Comment on attachment 8791343 [details] [diff] [review]
Follow up.

Review of attachment 8791343 [details] [diff] [review]:
-----------------------------------------------------------------

Sure, go for it. I do not know why we do not just include the whole file when it is in toolkit. Our about:troubleshooting page looks the same as FF, we just need to insert the account block. That would be a nice project :)
Attachment #8791343 - Flags: review?(acelists) → review+
(Assignee)

Comment 14

9 months ago
Thanks, landed follow-up:
https://hg.mozilla.org/comm-central/rev/39761f94a020
(DONTBUILD since there is no difference in functionality)
(Assignee)

Comment 15

9 months ago
Aurora (TB 50):
https://hg.mozilla.org/releases/comm-aurora/rev/605c8d4bcecc
https://hg.mozilla.org/releases/comm-aurora/rev/b684b3927470
status-thunderbird49: --- → affected
status-thunderbird50: --- → fixed
status-thunderbird51: --- → fixed
(Assignee)

Updated

9 months ago
No longer blocks: 1298085
(Assignee)

Updated

9 months ago
Attachment #8791243 - Flags: approval-comm-beta?
(Assignee)

Updated

9 months ago
status-thunderbird49: affected → wontfix
You need to log in before you can comment on or make changes to this bug.