Closed Bug 1454236 Opened 2 years ago Closed 2 years ago

Remove nsINode::eDOCUMENT_FRAGMENT

Categories

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

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

No description provided.
Comment on attachment 8968046 [details]
Bug 1454236: Remove nsINode::eDOCUMENT_FRAGMENT.

https://reviewboard.mozilla.org/r/236738/#review242466


Code analysis found 1 defect in this patch:
 - 1 defect found by clang-tidy

You can run this analysis locally with:
 - `./mach static-analysis check path/to/file.cpp` (C/C++)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: parser/html/nsHtml5TreeOperation.cpp:218
(Diff revision 1)
>  IsElementOrTemplateContent(nsINode* aNode)
>  {
>    if (aNode) {
>      if (aNode->IsElement()) {
>        return true;
> -    } else if (aNode->NodeType() == nsINode::DOCUMENT_FRAGMENT_NODE) {
> +    } else if (aNode->IsDocumentFragment()) {

Warning: Do not use 'else' after 'return' [clang-tidy: readability-else-after-return]
Comment on attachment 8968045 [details]
Bug 1454236: Introduce nsINode::{IsDocumentFragment, AsDocumentFragment}.

https://reviewboard.mozilla.org/r/236736/#review243862
Attachment #8968045 - Flags: review?(bzbarsky) → review+
Comment on attachment 8968046 [details]
Bug 1454236: Remove nsINode::eDOCUMENT_FRAGMENT.

https://reviewboard.mozilla.org/r/236738/#review243864

::: editor/libeditor/HTMLEditorDataTransfer.cpp:273
(Diff revision 1)
>                             nodeList,
>                             streamStartParent, streamStartOffset,
>                             streamEndParent, streamEndOffset);

Fix the weird preexisting indent here?

::: parser/html/nsParserUtils.cpp:167
(Diff revision 1)
>      rv = nsContentUtils::ParseFragmentHTML(
>        aFragment, fragment, nsGkAtoms::body, kNameSpaceID_XHTML, false, true);
>    }
>    if (fragment) {
>      nsTreeSanitizer sanitizer(aFlags);
> -    sanitizer.Sanitize(fragment);
> +    sanitizer.Sanitize(fragment->AsDocumentFragment());

This will look a bit saner once you merge on top of bug 1452183 (which is already landed).  In particular, `fragment` is a DocumentFragment after that, so the `sanitizer.Sanitize(fragment)` can stay as-is.  As in, please merge and take this change out.  ;)
Attachment #8968046 - Flags: review?(bzbarsky) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67ddc6418466
Introduce nsINode::{IsDocumentFragment, AsDocumentFragment}. r=bz
https://hg.mozilla.org/integration/mozilla-inbound/rev/658bffdfcc1d
Remove nsINode::eDOCUMENT_FRAGMENT. r=bz
https://hg.mozilla.org/mozilla-central/rev/67ddc6418466
https://hg.mozilla.org/mozilla-central/rev/658bffdfcc1d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.