Improve the functionality and correctness of the JEXL debugger page#225
Merged
Conversation
freshstrangemusic
force-pushed
the
freshstrangemusic/push-mzrkqsmqyukp
branch
6 times, most recently
from
June 26, 2026 16:08
54d5f7b to
d04c0a8
Compare
freshstrangemusic
changed the title
freshstrangemusic
force-pushed
the
freshstrangemusic/push-mzrkqsmqyukp
branch
from
July 7, 2026 22:35
d04c0a8 to
5e0b7cf
Compare
freshstrangemusic
changed the title
freshstrangemusic
force-pushed
the
freshstrangemusic/push-mzrkqsmqyukp
branch
2 times, most recently
from
July 7, 2026 22:58
4ff848c to
b244e55
Compare
Collaborator
Author
false expression display
attribute and pref warnings
context form
warning tooltip
|
freshstrangemusic
force-pushed
the
freshstrangemusic/push-mzrkqsmqyukp
branch
from
July 7, 2026 23:09
b244e55 to
7228ba3
Compare
RJAK11
approved these changes
Jul 8, 2026
RJAK11
left a comment
RJAK11
left a comment
Collaborator
There was a problem hiding this comment.
Some super tiny things but overall looks great to me!!
While here, I cleaned up all the jsdoc comments. We don't need to specify types in the comments since we're using TypeScript.
When trying to re-serialize an identifier AST, the code assumed that the
only possible LHS of a property access was a literal. However, mozjexl
supports accessors on object literals and on array literals (in which
case the attribute is accessed on the first element, or undefined is
returned).
This allows for parsing expressions of the form:
* `{x: 1}.x`
* `[{x: 1}].x`
Fixes #224
The serialization code for ObjectLiterals had a special case to handle nested literals. However, that case is unnecessary. This greatly simplifies the case.
Our string quoting algorithm was not particularily advanced -- it simply wrapped the result in string in single quotes. We now are using a more thorough algorithm that attempts to quote the string with any quote not used in the string and falls back to escaping single/double quotes (where appropriate). We also use this same algorithm when returning string values from `evaluateJexl()` so that the result can be used in the debugger verbatim. Fixes #228
If the expression is invalid, we can rely on evaluateExpression to throw and avoid doing the work to construct the AST (again) and recursively debug it. This also results in the errors from the evaluator instead of the parser.
Margins and padding have been tweaked to make everything line up with the fewest additional margin and padding classes. The `.large-checkbox` class was previously using `transform: scale(1.5)` to make the checkbox larger. However, `transform: scale(...)` is tricky as it also affects margin and padding. It ended up being simpler to change it to use constant height and width to make it larger and also aligned to the rest of the elements. Textareas (for object values) and text inputs (for strings) now take up the entire page width, which helps for editing bigger JSON payloads. All context inputs are now connected to their labels via DOM IDs, which means that clicking on the attribute name will focus the inputs. Field labels are now formatted monospace to increase readability.
Previously, an undefined value (i.e., the key is present in the context with an undefined value, or in code) would not render any form control. Now values will render as an object field, i.e., as a textbox that will attempt to parse as JSON. Parsing logic has been updated to special case the string `"undefined"` to parse to `undefined` and to stringify `undefined` to `"undefined"`. When the evaluator returns `undefined`, we no display an empty output section, but instead display `"undefined"`. Fixes #231
When we retieve the browser's targeting context we now also retieve the list of attributes are reported in Nimbus telemetry. We use this information to to highlight which context attributes are not reported in telemetry by bading each attribute with an exclamation mark. Hovering over the badge will show a tooltip informting the user that the context attribute is not available. Some renaming has taken place inside the JEXLDebuggerPage in order to clarify what exactly each piece of state contains: * `originalContext` has been renamed to `clientContext`, which mirrors the `ClientContext` type returned by `browser.experiments.nimbus.getClientContext()`; and * `modifiedContext` has been renamed to `contextOverrides` to make it clear that it only contains individually overidden keys and not the entire context. Fixes #221
The `debugJexl` (née `evaluateJexl`) now returns an object containing debugging information about the expression instead of a stringified representation of the display payload. `traverseAst` has been renamed to `collectFalseExprs` because we are now doing multiple additional traversals with `collectAttrsAndPrefs`. Fixes #230
freshstrangemusic
force-pushed
the
freshstrangemusic/push-mzrkqsmqyukp
branch
from
July 8, 2026 17:52
7228ba3 to
e20440f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adresses issues in the JEXL debugger:
undefinedvalues in the targeting context and output; andFixes #221
Fixes #224
Fixes #228
Fixes #230
Fixes #231