inDOMView contentRemoved implementation doesn't take into account hidden whitespace nodes

RESOLVED FIXED

Status

Other Applications
DOM Inspector
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: crussell, Assigned: crussell)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 443259 [details]
minimal test case

layout/inspector/src/inDOMView.cpp:965:
   if (container->GetChildCount() == 0) {
     // Fix up the parent
     parentNode->isContainer = PR_FALSE;
     parentNode->isOpen = PR_FALSE;
     mTree->InvalidateRow(NodeToRow(parentNode));
   }

Non-zero child count doesn't mean we're showing anything. If all of the children are ignorable whitespace, we need to update the parent row to no longer be a container.

1. Inspect the attached document, making sure "Show whitespace nodes" is off.
2. Open the BODY element
3. Delete the DIV

Note that the BODY element is still a container. You can even continue to toggle the open state, but there are no non-whitespace children to show.
Created attachment 446132 [details] [diff] [review]
Check inDOMViewNode's previous and next pointers for visible rows

Shawn, can you review this? If not, any suggestions? Boris? Neil?
Assignee: nobody → Sevenspade
Status: NEW → ASSIGNED
Attachment #446132 - Flags: review?(sdwilsh)
Comment on attachment 446132 [details] [diff] [review]
Check inDOMViewNode's previous and next pointers for visible rows

A layout peer is probably best, and this shouldn't need sr.
Attachment #446132 - Flags: review?(sdwilsh) → review?(roc)
Instead of letting previousNode and nextNode hang around, which looks slightly dangerous here since we're deleting stuff, how about setting a PRBool "isOnlyChild" and then testing it below after deleting stuff?
Created attachment 449568 [details] [diff] [review]
use an isOnlyChild PRBool instead of keeping pointers around
Attachment #446132 - Attachment is obsolete: true
Attachment #449568 - Flags: review?(roc)
Attachment #446132 - Flags: review?(roc)
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/a3eb83c7e386
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.