Closed Bug 1435666 Opened 8 years ago Closed 8 years ago

remove some SVG- and XPath-related XPCOM interfaces

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

Product:
Core
Component:
DOM: Core & HTML
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(4 files)

Bug 1435666 - Part 1: Remove nsIDOMSVGElement.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
Bug 1435666 - Part 2: Remove nsIDOMTimeEvent.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
Bug 1435666 - Part 3: Remove nsIDOMXPathResult.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
Bug 1435666 - Part 4: Remove nsIDOMXPathEvaluator.
59 bytes, text/x-review-board-request
bzbarsky
: review+
Details
No description provided.
:frg, please note. Like bug 1427512, this will at least need the package manifest updates to be ported across, and looking at comm-central there are some uses of constants on nsIDOMXPathResult that will need to be updated to either XPathResult.BLAH or using hard coded constant values.
Flags: needinfo?(frgrahl)
Or how about for calendar calendar/base/modules/calXMLUtils.jsm Instead of const XPR = Components.interfaces.nsIDOMXPathResult; we do: const XPR = { { // XPathResultType ANY_TYPE: 0, NUMBER_TYPE: 1, STRING_TYPE: 2, BOOLEAN_TYPE: 3, UNORDERED_NODE_ITERATOR_TYPE: 4, ORDERED_NODE_ITERATOR_TYPE: 5, UNORDERED_NODE_SNAPSHOT_TYPE: 6, ORDERED_NODE_SNAPSHOT_TYPE: 7, ANY_UNORDERED_NODE_TYPE: 8, FIRST_ORDERED_NODE_TYPE: 9 };
Depends on: 1435681
Thanks for the headup.
Flags: needinfo?(frgrahl)
Comment on attachment 8948315 [details] Bug 1435666 - Part 1: Remove nsIDOMSVGElement. https://reviewboard.mozilla.org/r/217792/#review223694 This is awesome. r=me ::: dom/svg/SVGTests.cpp:33 (Diff revision 1) > } > > already_AddRefed<DOMSVGStringList> > SVGTests::RequiredFeatures() > { > - nsCOMPtr<nsIDOMSVGElement> elem = do_QueryInterface(this); > + nsCOMPtr<nsIDOMElement> elem = do_QueryInterface(this); How about we QI to Element, then assert IsSVGElement() before doing the cast? Would make me a little happier about the casting situation here. Same elsewhere in this file. Alternatly, we could just have a pure virtual AsSVGElement() on SVGTests and implement it in the two subclasses (returning `this`), and skip the QI bits.
Attachment #8948315 - Flags: review?(bzbarsky) → review+
Comment on attachment 8948315 [details] Bug 1435666 - Part 1: Remove nsIDOMSVGElement. https://reviewboard.mozilla.org/r/217792/#review223704 Actually, one more thing. You need to fix mobile/android/installer/package-manifest.in too to remove dom_svg.xpt
Comment on attachment 8948316 [details] Bug 1435666 - Part 2: Remove nsIDOMTimeEvent. https://reviewboard.mozilla.org/r/217794/#review223706 Again, need to update mobile/android/installer/package-manifest.in. ::: dom/smil/nsSMILTimeValueSpec.cpp:375 (Diff revision 1) > } > > bool > nsSMILTimeValueSpec::CheckRepeatEventDetail(nsIDOMEvent *aEvent) > { > - nsCOMPtr<nsIDOMTimeEvent> timeEvent = do_QueryInterface(aEvent); > + TimeEvent* timeEvent = Event::FromSupports(aEvent)->AsTimeEvent(); I'd prefer you use aEvent->InternalDOMEvent() here, not Event::FromSupports.
Attachment #8948316 - Flags: review?(bzbarsky) → review+
Comment on attachment 8948317 [details] Bug 1435666 - Part 3: Remove nsIDOMXPathResult. https://reviewboard.mozilla.org/r/217796/#review223716 ::: devtools/client/webconsole/webconsole.js:986 (Diff revision 1) > // "filtered-by-type" class, which turns on or off the display. > > let attribute = !WORKERTYPES_PREFKEYS.includes(prefKey) > ? "filter" : "workerType"; > > - let xpath = ".//*[contains(@class, 'message') and " + > + let selector = "[" + attribute + "='" + prefKey + "'].message"; I guess we hope the attribute names and pref keys don't have any weird chars in them... but we had to hope that before too. ::: dom/base/test/unit/test_range.js:7 (Diff revision 1) > * 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/. */ > > const C_i = Components.interfaces; > > -const UNORDERED_TYPE = C_i.nsIDOMXPathResult.ANY_UNORDERED_NODE_TYPE; > +const UNORDERED_TYPE = 8; // XPathResult.ANY_UNORDERED_NODE_TYPE The other option is to expose XPathResult to importGlobalProperties. followup is fine for that.
Attachment #8948317 - Flags: review?(bzbarsky) → review+
Comment on attachment 8948317 [details] Bug 1435666 - Part 3: Remove nsIDOMXPathResult. https://reviewboard.mozilla.org/r/217796/#review223720 ::: dom/base/test/unit/test_range.js (Diff revision 1) > return do_parse_document(aPath, "application/xml").then(processParsedDocument); > } > > function processParsedDocument(doc) { > Assert.ok(doc.documentElement.localName != "parsererror"); > - Assert.ok(doc instanceof C_i.nsIDOMXPathEvaluator); This should be in the next changeset, right?
Comment on attachment 8948318 [details] Bug 1435666 - Part 4: Remove nsIDOMXPathEvaluator. https://reviewboard.mozilla.org/r/217798/#review223722 Again, update mobile/android/installer/package-manifest.in
Attachment #8948318 - Flags: review?(bzbarsky) → review+
Priority: -- → P2
Comment on attachment 8948317 [details] Bug 1435666 - Part 3: Remove nsIDOMXPathResult. https://reviewboard.mozilla.org/r/217796/#review223716 > I guess we hope the attribute names and pref keys don't have any weird chars in them... but we had to hope that before too. Yeah. In theory, the set of weird chars we'd behave unexpectedly on is different between XPath expressions and Selectors too, but hopefully it doesn't matter.
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #12) > The other option is to expose XPathResult to importGlobalProperties. > followup is fine for that. Filed bug 1436254.
Keywords: checkin-needed
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s efc5abe5c3956428b56f7958774bbb1c7f3519e1 -d fe90c7d53a7c: rebasing 446133:efc5abe5c395 "Bug 1435666 - Part 1: Remove nsIDOMSVGElement. r=bz" rebasing 446134:3421cf6041de "Bug 1435666 - Part 2: Remove nsIDOMTimeEvent. r=bz" rebasing 446135:420f16d6faf1 "Bug 1435666 - Part 3: Remove nsIDOMXPathResult. r=bz" merging browser/components/sessionstore/test/content-forms.js merging mobile/android/tests/browser/chrome/test_session_form_data.html merging testing/marionette/doc/api/element.js.html merging testing/marionette/element.js merging toolkit/modules/sessionstore/FormData.jsm rebasing 446136:fafee28d9e50 "Bug 1435666 - Part 4: Remove nsIDOMXPathEvaluator. r=bz" (tip) merging dom/base/nsDocument.cpp merging dom/base/nsIDocument.h merging layout/build/nsLayoutModule.cpp warning: conflicts while merging layout/build/nsLayoutModule.cpp! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Keywords: checkin-needed
Pushed by cam@mcc.id.au: https://hg.mozilla.org/integration/autoland/rev/5e7d8c3b3f91 Part 1: Remove nsIDOMSVGElement. r=bz https://hg.mozilla.org/integration/autoland/rev/8c1af668dd35 Part 2: Remove nsIDOMTimeEvent. r=bz https://hg.mozilla.org/integration/autoland/rev/5a15c3d2e334 Part 3: Remove nsIDOMXPathResult. r=bz https://hg.mozilla.org/integration/autoland/rev/be94b038da50 Part 4: Remove nsIDOMXPathEvaluator. r=bz
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/48b59ff51dca Port bug 1435666 to TB/IB/SM: remove dom_svg.xpt, dom_smil.xpt, dom_xpath.xpt from package manifests. rs=bustage-fix
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: