1281785 - JavaScript error: chrome://messenger/content/about-support/gfx.js, line 46: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[ISHIKAWA, Chiaki (back from day job exhibition event.)]

Since around late April of this year (2016), I have begun to see the
error message 10 times during local |make mozmill| test suite run with
full DEBUG build of C-C TB.

JavaScript error: chrome://messenger/content/about-support/gfx.js, line 46: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]


History:
I checked my local log. I did not see the messages in logs created on
April 1st and April 15th.  But I see them in a log created in April
24th.  I don't update source files daily.  But at least this history
can point to possible changes that may have occurred in that time
frame.

The error messages have persisted ever since up to the latest source
files I updated less than a day ago.

It looks to me someone somewhere calls
a string bundle function without a corresponding bundle item in the bundle.

TIA

Comment 1

[Sotaro Ikeda [:sotaro]]

It seems like regressions by Bug 1265496.

Comment 2

[Sotaro Ikeda [:sotaro]]

:dvander, can you comment to the bug?

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

It sounds like a localization string is missing. One of the calls to this function[1] probably is passing a string that no longer exists. bug 1265496 didn't remove any of these, but if I had a guess it'd be "clearTypeParameters" or "directWriteEnabled" causing trouble. If someone can figure out which one I'd just move it into the try block (or wrap the entire pushInfoRow function in a try block).

[1] https://dxr.mozilla.org/comm-central/source/mail/components/about-support/content/gfx.js#46

Comment 4

[Milan Sreckovic [:milan] (needinfo for best results)]

Is this still a problem in 48 beta?

Comment 5

[ISHIKAWA, Chiaki (back from day job exhibition event.)]

(In reply to Milan Sreckovic [:milan] from comment #4)
> Is this still a problem in 48 beta?

I have been on a summber holiday week, and so will refresh my source tree, report back.

TIA

Comment 6

[ISHIKAWA, Chiaki (back from day job exhibition event.)]

I refreshed the source tree a few hours ago and tested again, and still get the
error message.
10 such messages appeared in the |make mozmill| test suite run.
 
10 JavaScript error: chrome://messenger/content/about-support/gfx.js, line 46: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]

Comment 7

[Sotaro Ikeda [:sotaro]]

I take a look.

Comment 8

[Sotaro Ikeda [:sotaro]]

gfx.js problem was caused by the following.
 - GetStringFromName failures.
   + gfx.js still refers to "adapterDescription" instread of "gpuDescription".
      It is caused by change of bug 1265496.
   + "acceleratedWindows" was removed from "aboutSupport.properties" by Bug 1263849.
 - windowutils.layerManagerType failed because nsDOMWindowUtils::GetLayerManagerType() failure.

Comment 9

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Suppress gfx.js failures.

Comment 10

[Sotaro Ikeda [:sotaro]]

:milan, do we track this bug as firefox bug? gfx.js is not gecko source.
 https://dxr.mozilla.org/comm-central/source/mail/components/about-support/content/gfx.js

Comment 11

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8781939 [details] [diff] [review]
patch - Suppress gfx.js failures.

:dvander, the patch just suppress the gfx.js failures. Is it OK as a solution of the problem?

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Sotaro Ikeda [:sotaro] from comment #10)
> :milan, do we track this bug as firefox bug? gfx.js is not gecko source.
>  https://dxr.mozilla.org/comm-central/source/mail/components/about-support/
> content/gfx.js

I suppose it isn't really a Firefox bug, didn't notice that.

Comment 13

[ISHIKAWA, Chiaki (back from day job exhibition event.)]

(In reply to Sotaro Ikeda [:sotaro] from comment #11)
> Comment on attachment 8781939 [details] [diff] [review]
> patch - Suppress gfx.js failures.
> 
> :dvander, the patch just suppress the gfx.js failures. Is it OK as a
> solution of the problem?

This does not seem to be a firefox issue, but the patch posted should remove the warning from Thunderbird, correct?

TIA

Comment 14

[Sotaro Ikeda [:sotaro]]

(In reply to ISHIKAWA, Chiaki from comment #13)
> 
> This does not seem to be a firefox issue, but the patch posted should remove
> the warning from Thunderbird, correct?
> 
> TIA

Yes, the patch addressed the warning locally on my pc. But I am not sure that it is a way to go.

Comment 15

[Sotaro Ikeda [:sotaro]]

attachment 8781939 [details] [diff] [review] did not address the problem on windows. It needs to be updated.

Comment 16

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Suppress gfx.js failures

Addressed windows problem.

Comment 17

[rsx11m]

This patch needs review by a mail/ module owner or peer before it can land on comm-central.

Comment 18

[rsx11m]

Comment on attachment 8787061 [details] [diff] [review]
patch - Suppress gfx.js failures

Asking mkmelin on behalf of the assignee.

Comment 19

[Magnus Melin [:mkmelin]]

Comment on attachment 8787061 [details] [diff] [review]
patch - Suppress gfx.js failures

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

::: mail/components/about-support/content/gfx.js
@@ +46,5 @@
> +      try {
> +        string = bundle.GetStringFromName(name);
> +      } catch(e) {
> +        string = name;
> +      }

Isn't this unnecessary now when you fix the localization keys?

@@ +102,5 @@
> +    try {
> +      string = bundle.GetStringFromName(name);
> +    } catch(e) {
> +      string = name;
> +    }

Isn't this unnecessary now when you fix the localization keys?

@@ +261,5 @@
> +        acceleratedWindows++;
> +        mgrType = windowutils.layerManagerType;
> +      }
> +    }
> +    catch (e) {

nit: catch on the brace line like

} catch (e) {

Comment 20

[Sotaro Ikeda [:sotaro]]

(In reply to Magnus Melin from comment #19)
> Comment on attachment 8787061 [details] [diff] [review]
> patch - Suppress gfx.js failures
> 
> Review of attachment 8787061 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/components/about-support/content/gfx.js
> @@ +46,5 @@
> > +      try {
> > +        string = bundle.GetStringFromName(name);
> > +      } catch(e) {
> > +        string = name;
> > +      }
> 
> Isn't this unnecessary now when you fix the localization keys?
> 

Magnus, thanks for the review. It is still necessary since GPU2 related names were removed by Bug 1263849. How should them be handled? Current patch handle it just by adding try{}.

Comment 21

[Magnus Melin [:mkmelin]]

I don't understand. If they were removed, why would we be say calling pushInfoRow for the removed key? If it's removed, remove the caller, no?

Comment 22

[Sotaro Ikeda [:sotaro]]

(In reply to Magnus Melin from comment #21)
> I don't understand. If they were removed, why would we be say calling
> pushInfoRow for the removed key? If it's removed, remove the caller, no?

There are several modifications. In Bug 1263849, GPU names are split to different row like "GPU #1" and "GPU #2". And the names of "Direct2D" and "DirectWrite" are changed to non-localized name.
  https://dxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/aboutSupport.properties#53

Then information still exists but they does not take a name from aboutSupport.properties.

Comment 23

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Suppress gfx.js failures

Suppress the failures by addressing the comment.

Comment 24

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8790143 [details] [diff] [review]
patch - Suppress gfx.js failures

Magnus, how about this patch?

Comment 25

[Magnus Melin [:mkmelin]]

Comment on attachment 8790143 [details] [diff] [review]
patch - Suppress gfx.js failures

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

Looks ok to me, thx! r=mkmelin

::: mail/components/about-support/content/gfx.js
@@ +55,5 @@
> +      if (displayName) {
> +        string = displayName;
> +      } else {
> +        string = bundle.GetStringFromName(name);
> +      }

could be just

   let string = displayName || bundle.GetStringFromName(name);

@@ +111,5 @@
> +    if (displayName) {
> +      string = displayName;
> +    } else {
> +      string = bundle.GetStringFromName(name);
> +    }

same here

Comment 26

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Suppress gfx.js failures

Thanks! Applied the comment.

Comment 27

[rsx11m]

Pushed as http://hg.mozilla.org/comm-central/rev/77a72b7f61d4

Comment 28

[Jorg K (CEST = GMT+2)]

Why was that landed with "DONTBUILD"? Never mind, I'll land something on top:
https://hg.mozilla.org/comm-central/rev/44003aa9b4de

Comment 30

[Jorg K (CEST = GMT+2)]

Great job!! Finally "Help > Troubleshooting Information" gets populated again (will be 100% fixed in bug 1302651). Much appreciated. Thank you!

Comment 31

[Jorg K (CEST = GMT+2)]

Comment on attachment 8791047 [details] [diff] [review]
patch - Suppress gfx.js failures

This is already broken in TB 49, so let's uplift this to TB 50 (Aurora) at least. I don't think we're going TB 49 beta2.

Comment 32

[Jorg K (CEST = GMT+2)]

Actually, I tried this in 51.0a1 (2016-09-14) (64-bit) and I get this error in the Error log:

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

Can you take another look. In in local build though I don't get this error but instead run into bug 1302651. I'll refresh the M-C part of my local build and try again.

Comment 33

[Jorg K (CEST = GMT+2)]

Refreshed my local build and now I get:

JavaScript error: chrome://messenger/content/about-support/init.js, line 106: TypeError: Argument 1 of Node.appendChild is not an object.

Filed bug 1302701 for that.

Comment 34

[rsx11m]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #28)
> Why was that landed with "DONTBUILD"?

Because, if I hadn't, you would have complained about wasting CPU cycles a couple of hours before the nightly builds for a mail/-only patch which exclusively affects code that isn't covered by any tests.

For the sake of consistency, please update https://wiki.mozilla.org/Tree_Rules/comm-central if you want specific policies to be followed for checkin-needed patches.

Comment 35

[Jorg K (CEST = GMT+2)]

(In reply to rsx11m from comment #34)
> Because, if I hadn't, you would have complained about wasting CPU cycles a
> couple of hours before the nightly builds for a mail/-only patch which
> exclusively affects code that isn't covered by any tests.
The thought is much appreciated but you missed an important point:

This patch fixed the "Graphics" section in the "Troubleshooting Information". I wanted that in the next Daily. Sadly, if you land with DONTBUILD the next Daily runs on the *previous* push. Very sad[1]. I raised bug 1294415 for this but no one seems to be interested.

It's different for suite/ only changesets. There no build is triggered, but the next Daily *will* run on them.

[1] See C-A. The Daily of 9th Sept ran on 0f16ffc0589c of 3rd Sept despite my push ac425c17800f on 7th Sept.

Comment 36

[rsx11m]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #35)
> Sadly, if you land with DONTBUILD the next Daily runs on the *previous* push.

Eh, annoying indeed and good to know about this limitation. :-\

Comment 37

[Jorg K (CEST = GMT+2)]

Aurora (TB 50):
https://hg.mozilla.org/releases/comm-aurora/rev/11585df05353