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

RESOLVED FIXED in Thunderbird 51.0

Status

Thunderbird
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ISHIKAWA, Chiaki, Assigned: sotaro)

Tracking

({regression})

48 Branch
Thunderbird 51.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird49 wontfix, thunderbird50 fixed, thunderbird51 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

2 years ago
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
(Assignee)

Comment 1

2 years ago
It seems like regressions by Bug 1265496.
Blocks: 1265496
(Assignee)

Updated

2 years ago
Keywords: regression
(Assignee)

Comment 2

2 years ago
:dvander, can you comment to the bug?
Flags: needinfo?(dvander)
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
Flags: needinfo?(dvander)
status-firefox48: --- → fix-optional
Version: unspecified → 48 Branch
status-firefox47: --- → unaffected
status-firefox49: --- → affected
status-firefox50: --- → affected
Is this still a problem in 48 beta?
Flags: needinfo?(ishikawa)
(Reporter)

Comment 5

2 years ago
(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
Flags: needinfo?(ishikawa)
(Reporter)

Comment 6

2 years ago
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]
(Assignee)

Comment 7

2 years ago
I take a look.
(Assignee)

Comment 8

2 years ago
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.
(Assignee)

Updated

2 years ago
Blocks: 1263849
(Assignee)

Comment 9

2 years ago
Created attachment 8781939 [details] [diff] [review]
patch - Suppress gfx.js failures.
(Assignee)

Comment 10

2 years ago
: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
Flags: needinfo?(milan)
(Assignee)

Comment 11

2 years ago
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?
Attachment #8781939 - Flags: feedback?(dvander)
(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.
status-firefox48: fix-optional → unaffected
status-firefox49: affected → unaffected
status-firefox50: affected → unaffected
Flags: needinfo?(milan)
(Reporter)

Comment 13

2 years ago
(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
(Assignee)

Comment 14

2 years ago
(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.
Attachment #8781939 - Flags: feedback?(dvander) → review+
(Assignee)

Updated

2 years ago
status-firefox47: unaffected → ---
status-firefox48: unaffected → ---
status-firefox49: unaffected → ---
status-firefox50: unaffected → ---
Component: Graphics → General
Product: Core → Thunderbird
(Assignee)

Updated

2 years ago
Assignee: nobody → sotaro.ikeda.g
(Assignee)

Comment 15

2 years ago
attachment 8781939 [details] [diff] [review] did not address the problem on windows. It needs to be updated.
(Assignee)

Comment 16

2 years ago
Created attachment 8787061 [details] [diff] [review]
patch - Suppress gfx.js failures

Addressed windows problem.
Attachment #8781939 - Attachment is obsolete: true
Attachment #8787061 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 17

2 years ago
This patch needs review by a mail/ module owner or peer before it can land on comm-central.
Keywords: checkin-needed

Comment 18

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

Asking mkmelin on behalf of the assignee.
Attachment #8787061 - Flags: review+ → review?(mkmelin+mozilla)

Comment 19

2 years ago
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) {
(Assignee)

Comment 20

2 years ago
(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{}.
Flags: needinfo?(mkmelin+mozilla)

Updated

2 years ago
Blocks: 1265175

Comment 21

2 years ago
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?
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Comment 22

2 years ago
(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.
(Assignee)

Comment 23

2 years ago
Created attachment 8790143 [details] [diff] [review]
patch - Suppress gfx.js failures

Suppress the failures by addressing the comment.
Attachment #8787061 - Attachment is obsolete: true
Attachment #8787061 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 24

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

Magnus, how about this patch?
Attachment #8790143 - Flags: review?(mkmelin+mozilla)

Comment 25

2 years ago
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
Attachment #8790143 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 26

2 years ago
Created attachment 8791047 [details] [diff] [review]
patch - Suppress gfx.js failures

Thanks! Applied the comment.
Attachment #8790143 - Attachment is obsolete: true
Attachment #8791047 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 27

2 years ago
Pushed as http://hg.mozilla.org/comm-central/rev/77a72b7f61d4
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0

Comment 28

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

Updated

2 years ago
No longer blocks: 1265175
Duplicate of this bug: 1265175

Comment 30

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

Comment 31

2 years ago
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.
Attachment #8791047 - Flags: approval-comm-beta?
Attachment #8791047 - Flags: approval-comm-aurora+

Comment 32

2 years ago
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

2 years ago
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

2 years ago
(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

2 years ago
(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

2 years ago
(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

2 years ago
Aurora (TB 50):
https://hg.mozilla.org/releases/comm-aurora/rev/11585df05353
status-thunderbird49: --- → affected
status-thunderbird50: --- → fixed
status-thunderbird51: --- → fixed

Updated

2 years ago
Attachment #8791047 - Flags: approval-comm-beta?

Updated

2 years ago
status-thunderbird49: affected → wontfix

Updated

2 years ago
Depends on: 1303301
You need to log in before you can comment on or make changes to this bug.