Steps to reproduce: 1. Navigate to https://labs.jensimmons.com/2017/03-008.html. 2. Open the web inspector and enable the grid overlay for `main`. 3. Disable the authored margin on the `body` element. Expected Results: Labels would reposition to not be drawn outside the page. Actual Results: Labels are drawn outside the bounds of the page.
Created attachment 420491 [details] Screenshot of Issue
<rdar://problem/74648296>
Created attachment 424086 [details] Patch v1.0
Comment on attachment 424086 [details] Patch v1.0 View in context: https://bugs.webkit.org/attachment.cgi?id=424086&action=review > Source/WebKit/UIProcess/Inspector/ios/WKInspectorHighlightView.mm:340 > + CGPath* labelPath = labelWebCorePath.ensurePlatformPath(); I missed a `#if USE(CG)` here... And these two lines should move down to where they are used below the `textPosition` calculations.
Created attachment 424088 [details] Screenshot of Patch v1.0
Created attachment 424146 [details] Patch v1.1 - Added `USE(CG)` check
Comment on attachment 424146 [details] Patch v1.1 - Added `USE(CG)` check View in context: https://bugs.webkit.org/attachment.cgi?id=424146&action=review r=me, great stuff! > Source/WebKit/ChangeLog:8 > + Add support the new `WebCore::InspectorOverlay::LabelArrowEdgePosition` property to grid overlays on iOS. Nit: add support *for* > Source/WebKit/UIProcess/Inspector/ios/WKInspectorHighlightView.mm:411 > + auto labelWebCorePath = WebCore::InspectorOverlay::backgroundPathForLayoutLabel(textWidth + (padding * 2), textHeight + (padding * 2), arrowDirection, arrowEdgePosition, arrowSize); I would name this labelPath, and the existing labelPath as platformLabelPath
Created attachment 424374 [details] Patch v1.2 - Review notes
Created attachment 424376 [details] Patch v1.3 - Fix typo
ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!.
Created attachment 424436 [details] Patch v1.4 - Added reviewer to changelogs
Committed r275128: <https://commits.webkit.org/r275128> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424436 [details].
This introduced an unused variable warning: In file included from WebCore/DerivedSources/unified-sources/UnifiedSource-84c9f43f-3.cpp:4: ../../Source/WebCore/inspector/InspectorOverlay.cpp: In member function ‘WTF::Optional<WebCore::InspectorOverlay::Highlight::GridHighlightOverlay> WebCore::InspectorOverlay::buildGridOverlay(const WebCore::InspectorOverlay::Grid&, bool)’: ../../Source/WebCore/inspector/InspectorOverlay.cpp:1768:23: warning: variable ‘lineBetweenColumnBottoms’ set but not used [-Wunused-but-set-variable] 1768 | FloatLine lineBetweenColumnBottoms = { columnStartLine.end(), previousColumnEndLine.end() }; | It looks like a copy/paste error: FloatLine lineBetweenColumnTops = { columnStartLine.start(), previousColumnEndLine.start() }; FloatLine lineBetweenColumnBottoms = { columnStartLine.end(), previousColumnEndLine.end() }; gapLabelLine = { lineBetweenColumnTops.pointAtRelativeDistance(0.5), lineBetweenColumnTops.pointAtRelativeDistance(0.5) }; Patrick, can you look at it please?
There's also a new -Wreturn-type warning: ../../Source/WebCore/inspector/InspectorOverlay.cpp: In function ‘WebCore::FloatSize WebCore::expectedSizeForLayoutLabel(WTF::String, WebCore::InspectorOverlay::LabelArrowDirection, float)’: ../../Source/WebCore/inspector/InspectorOverlay.cpp:1349:54: warning: control reaches end of non-void function [-Wreturn-type] 1349 | auto font = InspectorOverlay::fontForLayoutLabel(); | ^ This is very common since clang doesn't warn when all enum values are covered by the switch, but GCC does. I'll add a RELEASE_ASSERT_NOT_REACHED().
> Patrick, can you look at it please? Yep, I’ll take a look and get these fixed today; thank you for bringing these to my attention!
I handled the -Wreturn-type in bug #224756, so that just leaves the copy/paste issue for you to handle.