Implement safe triangle behavior for interest invokers on menuitem elements. [chromium/src : main]
David Baron (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- Joey Arhar
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar added 9 comments![]( "Open in Gerrit")
void SetMenuSafeTriangle(MenuSafeTriangle*);
void ClearMenuSafeTriangle();Is SetMenuSafeTriangle(nullptr) redundant with ClearMenuSafeTriangle? If so, want to remove ClearMenuSafeTriangle? If not, want to add a comment explaining the difference?
// REVIEW: Should these be preserved as-is?How else would cancelable and behavior be preserved? Would we alternatively make this never cancelable?
// REVIEW: Should these be preserved as-is?Does "these" refer to the InterestState? How else would it be preserved?
void Recheck();A more descriptive name or a comment about when this is called and what it does would be nice
ContainerNode* item_parent = FlatTreeTraversal::Parent(*item);
if (IsA<HTMLFieldSetElement>(item_parent)) {
item_parent = FlatTreeTraversal::Parent(*item_parent);
}
if (!IsA<HTMLMenuListElement>(item_parent)) {
return;
}I thought that you'd be able to put divs and stuff in between these elements, but I didn't write the content model and I don't know what the current state of it is. Is there not a helper on the menuitem to find an ancestor menulist or menubar element?
// Have the current safe triangle recheck its conditions so that it goes
// away if needed.
current_safe_triangle->Recheck();if it doesn't go away, then at the end of this method it will be replace on the document by the new one. Then what happens to the old one?
// Maybe we shouldn't bother trying to use the last known mouse position
// when we're opening submenus via mechanisms other than mouse, and instead
// pass events through to this function (which is itself a bit of a pain).Yeah I was going to mention that using the current mouse position thing is kinda odd, but I figured that maybe there is no relevant event at all when you're hovering the cursor over an element for a certain amount of time, but I also don't know how interest invokers really works.
What even is the relevant event? I can see that passing it through ShowPopoverInternal could be annoying, but would it just be one new parameter for ShowPopoverInternal?
// If the submenu is fragmented, we don't do a safe triangle.Is this even possible? Either way I guess returning is probably the best thing to do.
Finish(true);want to put the parameter name as a comment?
Related details
- David Baron
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |