Avoid walking up frame tree across document boundaries in nsIFrame::IsVisibleConsideringAncestors and Accessible::VisibilityState
Categories
(Core :: Layout, enhancement, P2)
Tracking
()
Fission Milestone | M4 |
People
(Reporter: hiro, Assigned: hiro)
References
(Depends on 1 open bug, Blocks 2 open bugs)
Details
I've checked all call sites of StyleVisibility(), as of now, there are two call sites that the style is used to tell whether the frame is invisible or not across documents boundaries. They are nsIFrame::IsVisibleConsideringAncestors and Accessible::VisibilityState.
Instead of walking up the tree there, we can notify CSS visibility state changes to descendant documents and also we can notify the info when we switch frame in nsDeckFrame. I am going to split these part into other bugs for the safety, I mean, we can easily track down if it caused regressions.
As for nsView::GetVisibility(), the call sites of nsViewManager::SetViewVisibility (i.e. the place where we change the visibility of the given nsView) are in nsComboboxControlFrame.cpp, nsPluginFrame.cpp, nsMenuPopupFrame.cpp and nsFrame.cpp. I think the call sites other than the nsFrame.cpp don't need to be cared the across document boundary cases since each the nsView is for popup or dropdown or plugin which means it's in the same document or inside a plugin.
Assignee | ||
Comment 1•6 years ago
|
||
I did split the part that is to drop the nsLayoutUtils::GetCrossDocParentFrame call in Accessible::VisibilityState() as bug 1541705. The function also checks the given nsIFrame is scrolled out or not.
Comment 2•6 years ago
|
||
hiro, should this bug block bug 1518919? It seems like it should actually depend on it, right?
Assignee | ||
Comment 3•6 years ago
|
||
Oh right. As for CSS visibility stuff (bug 1518919), we just need bug 1541253 instead. I am fixing the dependency.
Assignee | ||
Updated•6 years ago
|
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
The last bit in this bug is nsView::GetVisiblity check in nsIFrame::IsVisibleConsideringAncestors and in Accessible::VisibilityState. (There is also nsLayoutUtils::IsPopup() check in Accessibile::VisibilityState, but I don't think we have cross-origin iframe as menu item in popup menu.)
Timothy, do we also need to do something for fission in terms of the GetVisibility check? I have totally no idea about nsView things.
Comment 5•5 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
The last bit in this bug is nsView::GetVisiblity check in nsIFrame::IsVisibleConsideringAncestors and in Accessible::VisibilityState. (There is also nsLayoutUtils::IsPopup() check in Accessibile::VisibilityState, but I don't think we have cross-origin iframe as menu item in popup menu.)
Timothy, do we also need to do something for fission in terms of the GetVisibility check? I have totally no idea about nsView things.
I'm kinda hoping we don't have to do anything here.
Views might get hidden via nsViewManager::SetViewVisibility, looking at the callers of that function we have:
- nsComboboxControlFrame
- nsPluginFrame
- nsMenuPopupFrame
- nsSubDocumentFrame
For 1,2, and 3 I haven't thought through it fully but I hope we don't need to do anything for them.
For 4 it is exactly in sync with the CSS property visibility. So we just need to check if a visibility hidden on an iframe "works" with these two functions.
Assignee | ||
Comment 6•5 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #5)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)
The last bit in this bug is nsView::GetVisiblity check in nsIFrame::IsVisibleConsideringAncestors and in Accessible::VisibilityState. (There is also nsLayoutUtils::IsPopup() check in Accessibile::VisibilityState, but I don't think we have cross-origin iframe as menu item in popup menu.)
Timothy, do we also need to do something for fission in terms of the GetVisibility check? I have totally no idea about nsView things.
I'm kinda hoping we don't have to do anything here.
Views might get hidden via nsViewManager::SetViewVisibility, looking at the callers of that function we have:
- nsComboboxControlFrame
- nsPluginFrame
- nsMenuPopupFrame
- nsSubDocumentFrame
For 1,2, and 3 I haven't thought through it fully but I hope we don't need to do anything for them.
For 4 it is exactly in sync with the CSS property visibility. So we just need to check if a visibility hidden on an iframe "works" with these two functions.
Great to know! Thanks!
I will close this bug once bug 1541256 and bug 1541705 are fixed.
Assignee | ||
Comment 7•5 years ago
|
||
Now bug 1541256 and bug 1541705 landed on m-c. Closing.
Description
•