86894 - Context menus etc. not namespace-aware

Status: VERIFIED FIXED

(Core :: XML, defect, P2)

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This applies mostly to XHTML. If you compare the behaviour of our context menus
and metadata on HTML and XHTML elements, you will notice things will not work in
XHTML if you have non-default namespace. The reason is that the context menus
and metadata use tagName property, when they should use localName.

<prefix:localname>

node.tagName = prefix:localname
node.localName = localname

I'll attach a fix, I expect some people on Cc: should be able to review.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix 1

Comment 2

[Blake Ross]

Cool. sr=blake

Comment 3

[Gervase Markham [:gerv]]

I may have rewritten metadata.js, but I don't understand namespaces at all. the
r= you need is definitely sicking's.

Gerv

Comment 4

[Johnny Stenback (:jst)]

Note that .localName may be null on certain nodes (doesn't work yet, but we'll
make this change soon, hopefully), so every call to
element.localName.toCaseUpper() n' friends need to do a check for a null
element.localName before calling toUpperCase().

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

In what situation can localName be null? Should I be using something else that
is guaranteed to return the tag name without prefix?

Comment 6

[Johnny Stenback (:jst)]

localName is (should be) null for every type of node except for Element and Attr
nodes, and even with those types of nodes localName *should* by the spec be null
if the element/attribute in question was created with a DOM Level 1 method, i.e.
document.createElement(), and not document.createElementNS(). I don't see how we
could possibly implement the latter (i.e. localName being null on some elements
and attributes), and IMO that doesn't make sense, but the former (i.e. localName
being null on non element and attribute nodes) will be implemented in mozilla.

Comment 7

[Johnny Stenback (:jst)]

Oh, and no, there's nothing else that is guaranteed to return the tagname w/o
the prefix in the DOM.

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I think metadata.js is ok, but I need to work a little on the context menu. We
are usually checking the node type (NODE_ELEMENT) before doing anything else.

Comment 9

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix 2 (check node.nodeType before accessing localName)

Comment 10

[Johnny Stenback (:jst)]

sr=jst

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

metadata.js should be ok wrt namespace, I spent a lot of effort making sure it 
was. However if elements would return NULL for localName it will throw an 
exception. Is this something we should worry about jst (sounds like no)?

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

grmbl. I just read the new metadata.js code, and it is not xhtml compatible :(((

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

So Jonas, could you review my changes to metadata.js?

Comment 14

[Johnny Stenback (:jst)]

Jonas, I don't think it's worth worrying about .localName ever being null on
*elements* in mozilla, if we choose to make that change somethime we'll have
lots of problems like these, things like this will be lost in the noice, should
we ever choose to go down that path :-)

What's not "xhtml compatible" in these changes?

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

r=sicking on the metadata.js changes

Comment 16

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed.

Comment 17

[Chris Petersen]

Marking verified per last comments

Comment 18

[Chris Petersen]

Compared a XHTML and HTML docunment that both contained a image file.
 
http://slip/projects/marvin/xhtml/transitional/img_align_middle.xml
http://slip/projects/marvin/html/img_align_middle.html


Right clicking on the image displayed the same popup menu.

Verified on the June 28th (2001062804) branch build.

Comment 19

[Heikki Toivonen (remove -bugzilla when emailing directly)]

nsBranch ok, safe.

Comment 20

[Nisheeth Ranjan]

The fix for this bug has been checked in on the trunk for more than a week and
is low risk.  QA has verified the fix on the trunk.  This is a high impact fix
for bringing the user experience of XHTML on par with HTML and allows us to
claim that our client supports the web's migration from HTML to XHTML.  Without
this fix, context menus did not display tag specific actions (for example, right
clicking on an image tag did not display the option to save the image).

Marking nsBranch+.

Comment 21

[Nisheeth Ranjan]

Steve granted this PDT+ status in today's PDT meeting.

Comment 22

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fix checked on branch as well.