[webkit-reviews] review denied: [Bug 34421] Web Inspector: Elements Panel: Limit the number of initially loaded element children : [Attachment 47851] [PATCH] Added limit checks for insertChild and removeChild

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 34421: Web Inspector: Elements Panel: Limit the number of initially loaded
element children
https://bugs.webkit.org/show_bug.cgi?id=34421

Attachment 47851: [PATCH] Added limit checks for insertChild and removeChild
https://bugs.webkit.org/attachment.cgi?id=47851&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
- "ask" is a strange prefix for the query parameter. As well as "are" is not
good for the push parameter.
- It might be better to use a query parameter 'maximimNodes' to return upon get
child nodes request.
- We should tell user how many nodes are missing (return that info and render
it).
- 500 should either be in cpp or js, not both. You either query for 500 or push
500.
- insertChild should not be muted when number of child nodes is equal to 500.
Where does this logic come from? In order to manage events properly, you should
extend the m_childrenRequested with the information on the number of child
nodes returned upon that request. That way you would be able to send only
events that make sense to the front-end.
- Imagine user is invoking 'Inspect element' on the 501-th child node. How many
siblings should we push? This makes it all a bit more complex. Should we
introduce 'collapsed ranges' of nodes? Or is it too challenging? I'd say for
now, if N-th node is requested, at least N sibling nodes should be pushed.
- I know I should have done it, but before we do something like this we need to
get elements panel tests.