Closed Bug 1739143 Opened 1 year ago Closed 1 year ago

"JavaScript error: , line 0: uncaught exception: undefined" if "data-l10n-id" points unexistent id

Categories

(Core :: Internationalization: Localization, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: arai, Assigned: zbraniecki)

References

Details

Attachments

(1 file)

Steps to reproduce:

  1. Add <label data-l10n-id="non-existent"/> to browser/components/preferences/dialogs/permissions.xhtml file, before </dialog> line
  2. ./mach build && ./mach run
  3. open about:preferences#privacy
  4. Click "Exceptions..." next to "Block pop-up windows" under "Permissions" section

Actual result:
the following is printed to console/terminal, and dialog isn't opened

JavaScript error: , line 0: uncaught exception: undefined

Expected result:
Some more helpful message that says the data-l10n-id is invalid value

Thank you!
I was able to reproduce and I'd love to improve that DX. Taking.

Assignee: nobody → zbraniecki
Status: NEW → ASSIGNED

:arai - can you verify that in browser console you in fact do see fluent specific warning?

I see:

[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent permissions.xhtml
[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent

I'm looking into why is it showing twice, but I wanted to check with you that you see it at least once :)

The JavaScript error: , line 0: uncaught exception: undefined is just because we don't catch the rejected promise, which we likely should, so I'll work on that.

Flags: needinfo?(arai.unmht)

Hmm, I think the double reporting was a weird artifact, I can't repro anymore.

As for undefined, the reason is that we coalesce a number of promises here: https://searchfox.org/mozilla-central/source/dom/l10n/DocumentL10n.cpp#129

and in case something doesn't translate completely this promise is rejected and uncaughts.

I think in case of DocumentL10n and DOMLocalization we should "silence" the rejection, because it is not actionable. We want to keep it for when TranslateElements or TranslateRoots is called from WebIDL API tho, so I'd like to know how to mark a promise as "ok to be rejected" and not report it.

:smaug - can you advise?

Flags: needinfo?(bugs)

Indeed, I overlooked the warning in browser console, sorry for that!
I was seeing the terminal's stdout/stderr, and then checked the browser console also has the error, but didn't notice the warnings.

what I get in the browser console is:

[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent permissions.xhtml
Uncaught (in promise) undefined
uncaught exception: undefined

and what I get in terminal is:

JavaScript error: , line 0: uncaught exception: undefined
JavaScript error: , line 0: uncaught exception: undefined

for each click.

so, the expected behavior about the helpful information is already there in browser console
just that there's extra error, and only that extra error is printed to terminal that's slightly confusing.

Flags: needinfo?(arai.unmht)

gotcha, yes! I'd like to fix that:

  1. Stop showing the uncaught promise (question to :smaug)
  2. Show the error in the terminal alongside console

For the (2) - :emilio, here's what I'm using now - https://searchfox.org/mozilla-central/source/intl/l10n/Localization.cpp#56-58 - is there a way to make it also dump to terminal alongside console?

Flags: needinfo?(emilio)

printf_stderr("%s\n", error.get()); or so?

Flags: needinfo?(emilio)

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #3)

I think in case of DocumentL10n and DOMLocalization we should "silence" the rejection, because it is not actionable. We want to keep it for when TranslateElements or TranslateRoots is called from WebIDL API tho, so I'd like to know how to mark a promise as "ok to be rejected" and not report it.

I'm not aware of such. Or do you perhaps mean calling StealExceptionFromJSContext on some ErrorResult and explicitly reject a promise?

Flags: needinfo?(bugs)

I guess the undefined is from here?
https://searchfox.org/mozilla-central/rev/6c8d325e61b0b445ed2e04899da38c3a4c266cba/dom/l10n/DocumentL10n.cpp#77

what's the expected consumer of the promise fulfilled/rejected there?
isn't the consumer supposed to handle the rejection somehow?

if we're completely not interested into the rejection, we could just call MaybeResolveWithUndefined() instead,
but maybe that's wrong way to go...

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f859ea95ec6
Capture DOM L10n initial translation and mutations rejections and report to console. r=nordzilla
Backout by abutkovits@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c53702d3deac
Backed out changeset 4f859ea95ec6 for causing failures at document_l10n/non-system-principal/browser_resource_uri.js. CLOSED TREE
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0d60e019527
Capture DOM L10n initial translation and mutations rejections and report to console. r=nordzilla
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch

Low prio but when you have a moment, can you verify that the DX is now matching your expectation?

Flags: needinfo?(zbraniecki) → needinfo?(arai.unmht)

Yes, it works perfectly!

I get the following in the terminal:

[fluent] Missing message in locale en-US: non-existent
[fluent] Couldn't find a message: non-existent
[dom/l10n] Could not complete initial document translation.

and the following in the browser console:

[fluent] Missing message in locale en-US: non-existent permissions.xhtml
[fluent] Couldn't find a message: non-existent permissions.xhtml
[dom/l10n] Could not complete initial document translation. permissions.xhtml

and the dialog opens, with the label with non-existent data-l10n-id untranslated.

Flags: needinfo?(arai.unmht)
Regressions: 1751836
You need to log in before you can comment on or make changes to this bug.