723431 - DOMTemplate should allow customisation of display of null/undefined values

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Joe Walker [:jwalker]]

Currently:

    <div id=a>${foo}</div>
        +
    domtemplate('a', { foo: null });
        V
    <div id=a>null</div>

This is good for debugging, but it's annoying - frequently you want null/undefined to show up as an empty string.

We should add an option so:

    <div id=a>${foo}</div>
        +
    domtemplate('a', { foo: null }, { blankNullUndefined: true });
        V
    <div id=a></div>

Comment 1

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=b9d6b2377f5a

Comment 2

[Joe Walker [:jwalker]]

Attachment: upload 1

Comment 3

[Joe Walker [:jwalker]]

If you'd like to see the extra parts cut-up github style: https://github.com/campd/gcli/pull/14

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 594990 [details] [diff] [review]
upload 1

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

::: browser/devtools/shared/Templater.jsm
@@ +54,5 @@
>   * @param node A DOM element or string referring to an element's id
>   * @param data Data to use in filling out the template
>   * @param options Options to customize the template processing. One of:
>   * - allowEval: boolean (default false) Basic template interpolations are
> + *   either property paths (e.g. ${a.b.c.d}), however if allowEval=true then we

This isn't new, but that 'either' seems to be missing an 'or'.

Comment 5

[Joe Walker [:jwalker]]

Attachment: upload 2

<blush>

I forgot to do the null/undefined processing on attribute values. See here for the fix:

           } else {
             // Replace references in all other attributes
             var newValue = value.replace(this._templateRegion, function(path) {
-              return this._envEval(path.slice(2, -1), data, value);
+              var insert = this._envEval(path.slice(2, -1), data, value);
+              if (this.options.blankNullUndefined && insert == null) {
+                insert = '';
+              }
+              return insert;
             }.bind(this));
             // Remove '_' prefix of attribute names so the DOM won't try
             // to use them before we've processed the template

Also this fixes your note about the either/or in the doc-comment, and adds tests for the above change.

Comment 6

[Joe Walker [:jwalker]]

https://bugzilla.mozilla.org/show_bug.cgi?id=723431

Comment 7

[Joe Walker [:jwalker]]

Attachment: upload 3

You reviewed this ages ago, but I never committed.

I re-pushed to try (https://tbpl.mozilla.org/?tree=Try&pusher=[e-mail]) and rolled in the one char fix to bug 736831, which you reviewed yesterday:
https://github.com/joewalker/gcli/commit/d4d93245aff9ad3cdd81733696443bd8a416844d and 
https://github.com/campd/gcli/pull/27#issuecomment-4883212

It seems like a no-brainer that I can commit this, but I'm just checking.

Comment 8

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=9d827d67bfbb

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

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

Comment 10

[(no longer active)]

This patch was in a range which caused a Ts regression, so I backed out the whole range:

https://hg.mozilla.org/mozilla-central/rev/24a6a53c714a

Please reland after investigating and fixing the regression.

Comment 11

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/d1195d800fde

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

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