Created attachment 232979 [details] [Screenshot] normal highlight When inspecting the DOM, highlights do not occur in about a 20px tall zone at the bottom of the window. To test this: 1. Open the 501nodes.html file attached. 2. Position a specific box at the bottom of the window. 3. Inspect and highlight that box.
<rdar://problem/17291737>
Created attachment 232981 [details] [Screenshot] Abnormal highlight
Created attachment 232983 [details] [Test] Nodes test
Created attachment 234130 [details] [PATCH] Proposed Fix Dealing with ScrollView / FrameView sizes is not my normal area of expertise. Simon / Benjamin, is there a better way to do this? In fact, the whole InspectorOverlay page approach could use an audit by someone who knows more about the page sizing. This fixes the issue in what I considered a safe way if there is no topContentInset.
Beth, you were recommended as a topContentInset expert by a few others. Does this patch look dirty to you, is there a better way, or some cleanup we can do in general here?
Comment on attachment 234130 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=234130&action=review > Source/WebCore/inspector/InspectorOverlay.cpp:350 > + frameViewFullSize.setHeight(frameViewFullSize.height() + view->topContentInset()); It this is for iOS, that makes no sense. If this is for OS X, that may make sense :)
(In reply to comment #6) > (From update of attachment 234130 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234130&action=review > > > Source/WebCore/inspector/InspectorOverlay.cpp:350 > > + frameViewFullSize.setHeight(frameViewFullSize.height() + view->topContentInset()); > > It this is for iOS, that makes no sense. > If this is for OS X, that may make sense :) This is OS X code right now. Node highlighting goes a different path for iOS.
Comment on attachment 234130 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=234130&action=review >> Source/WebCore/inspector/InspectorOverlay.cpp:350 >> + frameViewFullSize.setHeight(frameViewFullSize.height() + view->topContentInset()); > > It this is for iOS, that makes no sense. > If this is for OS X, that may make sense :) This code is only used on OS X right now.
Comment on attachment 234130 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=234130&action=review Hi Joe! Generally this concept behind this patch seems sound. But there are slightly better functions you can call to make it a little cleaner. > Source/WebCore/inspector/InspectorOverlay.cpp:348 > + viewportSize.setHeight(viewportSize.height() + view->topContentInset()); Instead of fetching visibleContentRect and manually adding in the content inset, you can just call unobscuredContentRect() > Source/WebCore/inspector/InspectorOverlay.cpp:349 > IntSize frameViewFullSize = view->visibleContentRectIncludingScrollbars().size(); And here instead of calling visibleContentRectIncludingScrollbars() and manually adding in the inset, you can call unobscuredContentRect(IncludeScrollbars)
(In reply to comment #9) > (From update of attachment 234130 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234130&action=review > > Hi Joe! Generally this concept behind this patch seems sound. But there are slightly better functions you can call to make it a little cleaner. > > > Source/WebCore/inspector/InspectorOverlay.cpp:348 > > + viewportSize.setHeight(viewportSize.height() + view->topContentInset()); > > Instead of fetching visibleContentRect and manually adding in the content inset, you can just call unobscuredContentRect() > > > Source/WebCore/inspector/InspectorOverlay.cpp:349 > > IntSize frameViewFullSize = view->visibleContentRectIncludingScrollbars().size(); > > And here instead of calling visibleContentRectIncludingScrollbars() and manually adding in the inset, you can call unobscuredContentRect(IncludeScrollbars) Going with these versions I end up with slightly smaller rects, and the highlights do get clipped: (lldb) p viewportSize // unobscuredContentRect (WebCore::IntSize) $16 = (m_width = 1218, m_height = 669) (lldb) p viewportSize1// visible + topContentInset (WebCore::IntSize) $17 = (m_width = 1218, m_height = 707) (lldb) p frameViewFullSize // unobscuredContentRect(ScrollableArea::IncludeScrollbars) (WebCore::IntSize) $18 = (m_width = 1218, m_height = 669) (lldb) p frameViewFullSize1 // visibleWithScrollbars + topContentInset (WebCore::IntSize) $19 = (m_width = 1218, m_height = 707) Is there something else that I may be missing?
> Going with these versions I end up with slightly smaller rects, and the highlights do get clipped: > > (lldb) p viewportSize // unobscuredContentRect > (WebCore::IntSize) $16 = (m_width = 1218, m_height = 669) > (lldb) p viewportSize1// visible + topContentInset > (WebCore::IntSize) $17 = (m_width = 1218, m_height = 707) > > (lldb) p frameViewFullSize // unobscuredContentRect(ScrollableArea::IncludeScrollbars) > (WebCore::IntSize) $18 = (m_width = 1218, m_height = 669) > (lldb) p frameViewFullSize1 // visibleWithScrollbars + topContentInset > (WebCore::IntSize) $19 = (m_width = 1218, m_height = 707) > > Is there something else that I may be missing? Oh jeez, the state of these functions is such a mess right now. Try: unscaledUnobscuredVisibleContentSize() and unscaledUnobscuredVisibleContentSize((ScrollableArea::IncludeScrollbars)
Different sizes, but still gets clipped: (lldb) p viewportSize // unscaledUnobscuredVisibleContentSize() (WebCore::IntSize) $14 = (m_width = 1218, m_height = 667) (lldb) p frameViewFullSize // unscaledUnobscuredVisibleContentSize((ScrollableArea::IncludeScrollbars) (WebCore::IntSize) $16 = (m_width = 1218, m_height = 667)
(In reply to comment #12) > Different sizes, but still gets clipped: > > (lldb) p viewportSize // unscaledUnobscuredVisibleContentSize() > (WebCore::IntSize) $14 = (m_width = 1218, m_height = 667) > > (lldb) p frameViewFullSize // unscaledUnobscuredVisibleContentSize((ScrollableArea::IncludeScrollbars) > (WebCore::IntSize) $16 = (m_width = 1218, m_height = 667) I'm sorry; I'm getting myself confused. Of course we don't want the unobscured rects. Please try: unscaledTotalVisibleContentSize() unscaledTotalVisibleContentSize(ScrollableArea::IncludeScrollbars))
Created attachment 234191 [details] [PATCH] Proposed Fix Perfect!
Comment on attachment 234191 [details] [PATCH] Proposed Fix Clearing flags on attachment: 234191 Committed r170665: <http://trac.webkit.org/changeset/170665>
All reviewed patches have been landed. Closing bug.