Use createElementNS instead of createElement for XUL in printoptions

RESOLVED WONTFIX

Status

()

defect
--
minor
RESOLVED WONTFIX
15 years ago
9 years ago

People

(Reporter: timeless, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(1 attachment)

Reporter

Description

15 years ago
this is a code audit of sorts based on an irc discussion. if you are not more
familiar than I am with DOM2/DOM3 then you probably should read about it before
deciding to work on this task. it's probably also a good idea to read bug 280692
which was filed based on the irc conversation.
Assignee: printing → nobody
QA Contact: printing
Posted patch PatchSplinter Review
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
Attachment #430947 - Flags: review?(neil)
Neil: ping?
Blocks: 280692
No longer depends on: 280692
OS: Windows XP → All
Hardware: x86 → All
Attachment #430947 - Flags: review?(neil) → review?(roc)
Comment on attachment 430947 [details] [diff] [review]
Patch

I am not a toolkit peer
Attachment #430947 - Flags: review?(roc) → review?(gavin.sharp)
Comment on attachment 430947 [details] [diff] [review]
Patch

I'm pretty sure this file has no test coverage. Have you tested these changes manually? It's not particularly difficult to verify the correctness of these changes by inspection, obviously, but bugs have been caused by simpler changes when there's no testing involved!
Attachment #430947 - Flags: review?(gavin.sharp) → review+
Comment on attachment 430947 [details] [diff] [review]
Patch

Manual testing didn't reveal breakage. Thanks, Gavin.
Attachment #430947 - Flags: approval2.0?
Err, bug 280692 seems dead, the latest comment suggests WONTFIX. There are also many more places that we'd have to change if bug 280692 would be resurrected for some reason. What's the point of landing this patch, now?
Keywords: checkin-needed
Henri's argument is that we shouldn't change HTML namespace -> null namespace, as was the original intention in bug 280692; we should still change XUL namespace -> HTML namespace, though.

Also, it would be good to land this patch in any case, as its approach is more robust.
Keywords: checkin-needed
(In reply to comment #7)
> we should still change XUL namespace -> HTML namespace, though.

Why would we want to do this for XUL documents as opposed to HTML documents, and where's the bug for this? Where are the bugs for "fixing" other uses of createElement in our code base?

> Also, it would be good to land this patch in any case, as its approach is more
> robust.

It doesn't provide any win in robustness. This is a script very much tailored for running in one specific XUL document.
Comment on attachment 430947 [details] [diff] [review]
Patch

Let's not land it now. WONTFIX seems appropriate.
Attachment #430947 - Flags: approval2.0+ → approval2.0-
Assignee: Ms2ger → nobody
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.