1729255 - Avoid blank debugger tab when loading watch expression with special characters

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Jon Scharff]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

When opening the devtools debug tab, it is blank. As per the suggestions from :jdescottes (https://bugzilla.mozilla.org/show_bug.cgi?id=1464132), I created a new profile, and the problem did not occur. I also showed the browser console, and an JSON parse error seems to occur when trying to display the debug tab contents (see "What happened?" for the stack trace). If possible, I would like to find out what the offending JSON is and fix/remove it.

I am using the Japanese localized of version 91.0.2 (64 bit). I don't remember the version it was last working in, but it did work at some point last month, at the very least, and I let the auto-update do its thing.

Actual results:

SyntaxError: JSON.parse: bad character in string literal at line 1 column 105 of the JSON data
get resource://devtools/client/shared/prefs.js:146
restoreExpressions resource://devtools/client/debugger/src/reducers/expressions.js:108
initialExpressionState resource://devtools/client/debugger/src/reducers/expressions.js:25
update resource://devtools/client/debugger/src/reducers/expressions.js:33
Redux 3
bootstrapStore resource://devtools/client/debugger/src/utils/bootstrap.js:75
bootstrap resource://devtools/client/debugger/src/main.js:97
Promise-backend.js:955:35

Expected results:

The debugger tab should have shown.

Comment 1

[Julian Descottes [:jdescottes]]

Thanks for logging, could you open about:config, search for devtools.debugger.expressions and copy the value you have for this preference?
Note that clearing the preference will probably get the debugger back to work but it would be great to get the value first!

Comment 2

[Julian Descottes [:jdescottes]]

Ok I think I managed to get it as well by creating the following expression:

getString("？")

The question mark here is the one we get when using a japanese keyboard.
The preference seems to store the following:

[{"input":"getString(\"\")","updating":false}]

Keeping the needinfo to check that the issue is similar.

Comment 3

[Jon Scharff]

[{"input":"date","updating":false},{"input":"input.filter(function (sj) { return isDetailComplete(sj, \"\", \"EndTime\", date) && !isDetailComplete(sj, \"W\", \"StartTime\", date); })","updating":false}]

looks like an encoding issue? When I preview, the "garbage" characters disappear, so they may be removed by the comment processing... I'll try to add a text file with the above content as well.

Comment 4

[Jon Scharff]

Attachment: devtools.debugger.expressions contents

Comment 5

[Julian Descottes [:jdescottes]]

Thanks, this looks like a similar issue to what I saw. Feel free to click on the "<-" icon on the right of the devtools.debugger.expression to restore it to the default value. It should fix your debugger UI for now.

Keeping the bug for triage later this week.

Comment 6

[Jon Scharff]

Attachment: devtools.debugger.expressions contents (pasted into notepad and saved as utf-8)

Comment 7

[Jon Scharff]

Julian, resetting the value solved the issue. Thanks, and let me know if I can do anything else to help with this!

Comment 8

[Julian Descottes [:jdescottes]]

P2/S3 because panel is blank but it's not a recent regression

Comment 9

[Julian Descottes [:jdescottes]]

Let's make this bug about fixing the symptoms (wrap in try catch + test maybe?), and file a follow up to encode all expressions properly.

Comment 10

[Julian Descottes [:jdescottes]]

Seems to work on ESR 78, would be nice to know when this started failing.

Comment 11

[Julian Descottes [:jdescottes]]

STRs for finding the regression window:

• open DevTools
• select Debugger panel
• in the right panel, under "Watch Expressions" create an expression for getString("？") (copy the exact string, the question mark is a special character here)
• close DevTools, reopen DevTools

ER: The Debugger UI should be displayed
AR: The Debugger UI is blank. If you need to fix the profile after that, reset the devtools.debugger.expressions preference.

Comment 12

[Timea Cernea [:tbabos][inactive]]

Mozregression revealed Bug 1668117 to be the regressor of this issue.
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d548bb7ebca0a5fc219a31e570baa28605837fcd&tochange=ba4685a82eea5c38acf60ac093b52ffc2c81ef97

Comment 13

[Julian Descottes [:jdescottes]]

Thanks for finding the regressing bug!

I guess there must be a difference between the PrefsHelper that was defined on GitHub https://github.com/firefox-devtools/devtools-core/blob/master/packages/devtools-modules/src/prefs.js and the one we have locally.

Will take a look.

Comment 15

[Julian Descottes [:jdescottes]]

Attachment: Bug 1729255 - [devtools] Store JSON preferences as String instead of Char in PrefsHelper

JSON preferences used to be stored in string preferences, which is compatible with special characters.
There should not be any data loss, char prefs seem to gracefully transition to string prefs.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f2fc83728a24
[devtools] Store JSON preferences as String instead of Char in PrefsHelper r=bomsy

Comment 17

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/f2fc83728a24

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jdescottes, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 19

[Julian Descottes [:jdescottes]]

Given that this is broken since 83, I don't think it's worth uplifting to 93. It can ride the trains. Marking as wontfix for 93.

For ESR91 I'm not sure though. ESR users will be stuck with this bug for a long time so it could be nice to uplift it to esr91

Comment 20

[Ryan VanderMeulen [:RyanVM]]

(In reply to Julian Descottes [:jdescottes] from comment #19)

For ESR91 I'm not sure though. ESR users will be stuck with this bug for a long time so it could be nice to uplift it to esr91

Please nominate if you feel it should be considered for uplift.

Comment 21

[Julian Descottes [:jdescottes]]

Comment on attachment 9241656 [details]
Bug 1729255 - [devtools] Store JSON preferences as String instead of Char in PrefsHelper

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: If a user saves a watch expression with special characters, they will be left with a broken debugger with no way to recover from it. For a developer who is forced to use ESR, they would have to wait until mid 2022 to get back a working debugger UI.
• User impact if declined: Users might be locked out of using the Debugger
• Fix Landed on Version: 94
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small javascript change, unit tested, no regression since landing the patch in FF94
• String or UUID changes made by this patch:

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9241656 [details]
Bug 1729255 - [devtools] Store JSON preferences as String instead of Char in PrefsHelper

Approved for 91.4esr.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/5aeb97e86758