Bug 1368408 Selection::SelectFrames() can assume that the result of nsRange::GetStartParent(), nsRange::GetEndParent() and nsIContentIterator::GetCurrentNode() never returns nullptr in it r?mats
Selection::SelectFrames() is an internal method of Selection, it always handles positioned range. So, neither aRange->GetStartParent() nor aRange->GetEndParent() never returns nullptr.
Additionally, nsIContentIterator::GetCurrentNode() shouldn't return nullptr until IsDone() returns true.
MozReview-Commit-ID: 1IS4nMLukt
--- a/layout/generic/nsSelection.cpp
+++ b/layout/generic/nsSelection.cpp
@@ -4497,21 +4497,18 @@ Selection::SelectAllFramesForContent(nsI
}
if (NS_WARN_IF(NS_FAILED(aInnerIter->Init(aContent)))) {
return NS_ERROR_FAILURE;
}
for (; !aInnerIter->IsDone(); aInnerIter->Next()) {
nsINode* node = aInnerIter->GetCurrentNode();
- // Detect the bug of content iterator, but shouldn't cause a crash in
- // release builds.
MOZ_ASSERT(node);
- nsIContent* innercontent =
- node && node->IsContent() ? node->AsContent() : nullptr;
+ nsIContent* innercontent = node->IsContent() ? node->AsContent() : nullptr;
SelectFramesForContent(innercontent, aSelected);
}
return NS_OK;
}
/**
* The idea of this helper method is to select or deselect "top to bottom",
@@ -4520,36 +4517,39 @@ Selection::SelectAllFramesForContent(nsI
nsresult
Selection::SelectFrames(nsPresContext* aPresContext, nsRange* aRange,
bool aSelect)
{
if (!mFrameSelection || !aPresContext || !aPresContext->GetPresShell()) {
// nothing to do
return NS_OK;
}
- MOZ_ASSERT(aRange);
+ MOZ_ASSERT(aRange && aRange->IsPositioned());
if (mFrameSelection->GetTableCellSelection()) {
nsINode* node = aRange->GetCommonAncestor();
nsIFrame* frame = node->IsContent() ? node->AsContent()->GetPrimaryFrame()
: aPresContext->FrameManager()->GetRootFrame();
if (frame) {
frame->InvalidateFrameSubtree();
}
return NS_OK;
}
// Loop through the content iterator for each content node; for each text
// node, call SetSelected on it:
nsINode* startNode = aRange->GetStartParent();
nsIContent* startContent =
- startNode && startNode->IsContent() ? startNode->AsContent() : nullptr;
+ startNode->IsContent() ? startNode->AsContent() : nullptr;
if (!startContent) {
// Don't warn, bug 1055722
+ // XXX The range can start from a document node and such range can be
+ // added to Selection with JS. Therefore, even in such cases,
+ // shouldn't we handle selection in the range?
return NS_ERROR_UNEXPECTED;
}
// We must call first one explicitly
bool isFirstContentTextNode = startContent->IsNodeOfType(nsINode::eTEXT);
nsINode* endNode = aRange->GetEndParent();
if (isFirstContentTextNode) {
nsIFrame* frame = startContent->GetPrimaryFrame();
@@ -4586,28 +4586,28 @@ Selection::SelectFrames(nsPresContext* a
nsCOMPtr<nsIContentIterator> iter = NS_NewContentSubtreeIterator();
iter->Init(aRange);
if (isFirstContentTextNode && !iter->IsDone()) {
iter->Next(); // first content has already been handled.
}
nsCOMPtr<nsIContentIterator> inneriter = NS_NewContentIterator();
for (; !iter->IsDone(); iter->Next()) {
nsINode* node = iter->GetCurrentNode();
- // Detect the bug of content iterator, but shouldn't cause a crash in
- // release builds.
MOZ_ASSERT(node);
- nsIContent* content =
- node && node->IsContent() ? node->AsContent() : nullptr;
+ nsIContent* content = node->IsContent() ? node->AsContent() : nullptr;
SelectAllFramesForContent(inneriter, content, aSelect);
}
// We must now do the last one if it is not the same as the first
if (endNode != startNode) {
nsIContent* endContent =
- endNode && endNode->IsContent() ? endNode->AsContent() : nullptr;
+ endNode->IsContent() ? endNode->AsContent() : nullptr;
+ // XXX The range can end at a document node and such range can be
+ // added to Selection with JS. Therefore, even in such cases,
+ // shouldn't we handle selection in the range?
if (NS_WARN_IF(!endContent)) {
return NS_ERROR_UNEXPECTED;
}
if (endContent->IsNodeOfType(nsINode::eTEXT)) {
nsIFrame* frame = endContent->GetPrimaryFrame();
// The frame could be an SVG text frame, in which case we'll ignore it.
if (frame && frame->IsTextFrame()) {
nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);