Closed
Bug 1452352
Opened 6 years ago
Closed 6 years ago
Port bug 1452235 - Remove nsIDOMSerializer
Categories
(MailNews Core :: General, defect)
MailNews Core
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 61.0
People
(Reporter: jorgk-bmo, Assigned: Paenglab)
References
Details
Attachments
(1 file, 3 obsolete files)
3.02 KB,
patch
|
Paenglab
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1452235 +++ Boris will be removing the "@mozilla.org/xmlextras/xmlserializer;1" and replacing it with ChromeUtils.importGlobalProperties(["XMLSerializer"]); followed by |new XMLSerializer()|. There look like one use each in calendar and mailnews that will need updating. Richard, could to take this?
Flags: needinfo?(richard.marti)
Assignee | ||
Comment 1•6 years ago
|
||
Jörg, is this what you thought? I haven't tested it.
Flags: needinfo?(richard.marti)
Attachment #8965938 -
Flags: review?(jorgk)
Reporter | ||
Comment 2•6 years ago
|
||
Comment on attachment 8965938 [details] [diff] [review] nsIDOMSerializer.patch Review of attachment 8965938 [details] [diff] [review]: ----------------------------------------------------------------- Thanks. ::: calendar/base/modules/calXMLUtils.jsm @@ +162,5 @@ > * @param doc The DOM document to serialize > * @return The DOM document as a string. > */ > cal.xml.serializeDOM = function(doc) { > + let serializer = new XMLSerializer()); You have two closing parenthesis here, at the other call site as well.
Attachment #8965938 -
Flags: review?(philipp)
Attachment #8965938 -
Flags: review?(jorgk)
Attachment #8965938 -
Flags: review+
Assignee | ||
Comment 3•6 years ago
|
||
Fixed the parenthesis.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8965942 -
Flags: review?(philipp)
Assignee | ||
Updated•6 years ago
|
Attachment #8965938 -
Attachment is obsolete: true
Attachment #8965938 -
Flags: review?(philipp)
Comment 4•6 years ago
|
||
Comment on attachment 8965942 [details] [diff] [review] nsIDOMSerializer.patch Review of attachment 8965942 [details] [diff] [review]: ----------------------------------------------------------------- r+ with these changes: ::: calendar/base/modules/calXMLUtils.jsm @@ +10,1 @@ > Components.utils.importGlobalProperties(["XMLHttpRequest"]); You can just combine this to Cu.importGlobalProperties(["XMLSerializer", "XMLHttpRequest"]); I believe importGlobalProperties is not available on ChromeUtils ::: mailnews/extensions/newsblog/content/feed-parser.js @@ +2,5 @@ > * This Source Code Form is subject to the terms of the Mozilla Public > * License, v. 2.0. If a copy of the MPL was not distributed with this > * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > > +ChromeUtils.importGlobalProperties(["XMLSerializer"]); Same here, you need to use Cu instead.
Attachment #8965942 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 5•6 years ago
|
||
Changed to Cu. I also changed the line Components.utils.importGlobalProperties(["XMLHttpRequest"]); to Cu.importGlobalProperties(["XMLHttpRequest"]); in calXMLUtils.jsm.
Attachment #8965942 -
Attachment is obsolete: true
Attachment #8965945 -
Flags: review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #4) > You can just combine this to > Cu.importGlobalProperties(["XMLSerializer", "XMLHttpRequest"]); Philipp wanted one line here, sorry.
Assignee | ||
Comment 7•6 years ago
|
||
Ah, now I see how this was meant. Sorry, not the best on JS.
Attachment #8965945 -
Attachment is obsolete: true
Attachment #8965961 -
Flags: review+
Assignee | ||
Comment 8•6 years ago
|
||
I remove the c-n as it would break TB without bug 1452235 landed (not checked with bug 1452235 but without).
Keywords: checkin-needed
Reporter | ||
Comment 9•6 years ago
|
||
Hmm, I've asked Boris in bug 1452235 comment #7. We have this already: mailnews/extensions/newsblog/content/feed-subscriptions.js 2357 let serializer = new XMLSerializer(); So I'm unsure, but I wasn't going to land it today.
Assignee | ||
Comment 10•6 years ago
|
||
Maybe it's because of the Cu.importGlobalProperties. With the patch applied I couldn't open the console.
Comment 11•6 years ago
|
||
You can find the list of supported properties for Cu.importGlobalProperties here: https://searchfox.org/comm-central/source/mozilla/js/xpconnect/src/Sandbox.cpp#801 it doesn't contain XMLSerializer yet, so it would break. using new XMLSerializer() already works in DOM windows, just not in jsms etc, which is what Cu.importGlobalProperties is for.
Comment 13•6 years ago
|
||
Pushed by richard.marti@gmail.com: https://hg.mozilla.org/comm-central/rev/78c17f98345c Port bug 1452235: Replace use of nsIDOMSerializer. r=jorgk
Assignee | ||
Updated•6 years ago
|
Target Milestone: --- → Thunderbird 61.0
Reporter | ||
Comment 14•6 years ago
|
||
The change in calendar/modules/utils/calXMLUtils.jsm got overwritten by recent Calendar refactoring, so doing it again :-(
Comment 15•6 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/c5068dc7ac72 Port bug 1452235: Replace use of nsIDOMSerializer (follow-up). r=me
You need to log in
before you can comment on or make changes to this bug.
Description
•