1239183 - Use XHTML as the default namespace in webconsole.xul

Status: RESOLVED INVALID

(DevTools :: Console, defect)

Description

[Lin Clark [:linclark]]

See META: Bug 1239038

Comment 1

[Lin Clark [:linclark]]

Attachment: Bug1239183.patch

This patch:
- switches the namespace in webconsole.xul
- changes from createElement() to createElementNS() in client/webconsole/ and shared/webconsole/
- switches conditionals from checking against tagName to using localName... tagName includes the prefix. It would be ok to use that value if the namespace were expanded. However, hardcoding to assume a particular prefix breaks the indirection of namespace prefixing. See WHATWG CP-120 (https://wiki.whatwg.org/wiki/Change_Proposal_for_ISSUE-120)

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb5d1663c379

Comment 2

[James Long (:jlongster)]

Wowza, that was quick. Thanks! I'm going to comment in bug 1239183 about this.

Comment 3

[James Long (:jlongster)]

So we may not need to change the default namespace, if all that does is change what you need to prepend on the static elements in the XUL/XHTML file. But if we go the route of patching `createElement`, we definitely should go through and make sure everything is using `createElementNS` in the old code.

Comment 4

[James Long (:jlongster)]

(oops, I meant "comment in bug 1238881")

Comment 5

[James Long (:jlongster)]

Comment on attachment 8707270 [details] [diff] [review]
Bug1239183.patch

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

No r+ as it stands because of the changes in webconsole.xul, we don't actually need to do that, since it doesn't change anything. Sorry about that.

Depending on how we make React work in a XUL context, we still might need all the other changes though. I'll follow-up in this bug soon.

::: devtools/client/webconsole/test/browser_webconsole_bug_653531_highlighter_console_helper.js
@@ +13,5 @@
>  
>  function createDocument() {
>    let doc = content.document;
>    let div = doc.createElement("div");
> +  h1 = doc.createElementNS(XHTML_NS, "h1");

Wasn't it creating XUL elements before? The difference is how styles are applied. A `h1` XUL element has different default styles than a `h1` HTML element.

Shouldn't these be XUL_NS instead, unless you've looked at them and definitely know it doesn't break anything?

::: devtools/client/webconsole/webconsole.xul
@@ +13,5 @@
>                   type="text/css"?>
>  <?xml-stylesheet href="chrome://devtools/skin/webconsole.css"
>                   type="text/css"?>
>  <?xul-overlay href="chrome://global/content/editMenuOverlay.xul"?>
> +<xul:window xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"

I was wrong about changing this default namespace... it doesn't help `document.createElement`. So while we might need to still convert all `createElement` calls to explicit `createElementNS` calls in other files if we're going to override `createElement`, I don't think we need to make the changes in this file.

Sorry for going down the wrong path!

Comment 6

[Lin Clark [:linclark]]

> Shouldn't these be XUL_NS instead, unless you've looked at them and definitely know it doesn't break anything?

I didn't compare them visually, so you might be right.

> Sorry for going down the wrong path!

Not a problem, it was worth an experiment :)

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

webconsole.xul does not exist anymore