Created attachment 250363 [details] Top focus ring location is correct, Bottom focus ring within modal is wrong Environment: Safari Version 8.0.4 (10600.4.10.7) Steps to reproduce: 1. Open http://jsbin.com/tufoye/2/edit 2. Resize the Output pane to be at least 800px wide. 3. Hover over the lower-left part of the gray rectangle where the mouse cursor becomes the gloved-hand pointer. (Which is to say that you're hovering over the image map's <area>.) 4. Click. 5. Note the location of the circular blue focus ring that appears. 6. Click the "Launch demo modal" button. 7. In the modal box that appears, click the same location in its gray rectangle image map. 8. Note the location of the circular blue focus ring that appears. Expected result: The locations of the focus rings in steps 5 & 8 should be the same relative to their respective gray rectangle image maps. Actual result: The location of the focus ring in step 8 is incorrect and differs from that of the focus ring in step 5. Original Bootstrap bug report: https://github.com/twbs/bootstrap/issues/16180
Corresponding Chrome/Blink bug: https://code.google.com/p/chromium/issues/detail?id=475128
Created attachment 252501 [details] Copy of JS Bin example
Created attachment 255651 [details] Example An example that demonstrates the issue.
Created attachment 255652 [details] Example
HTMLAreaElement::computePath() has: // FIXME: This doesn't work correctly with transforms. FloatPoint absPos = obj->localToAbsolute();
HTMLAreaElement needs to not blithely return absolute paths; I think the caller should do the absolute conversion if they need to.
<rdar://problem/21908735>
Created attachment 275968 [details] Patch
Comment on attachment 275968 [details] Patch Attachment 275968 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1118347 New failing tests: fast/images/image-map-outline.html fast/images/imagemap-focus-ring-zoom-style.html
Created attachment 275977 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 275980 [details] Patch
Comment on attachment 275980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275980&action=review Looks like all non-Mac builds are failing. > Source/WebCore/html/HTMLAreaElement.h:48 > + Path pathForFocusRing(const LayoutSize& elementSize) const { return getRegion(m_shape == Default ? elementSize : m_lastSize); } Does this really have to be inlined? I suggest defining it in the cpp file instead of here. Can we paragraph this with computePath rather than with imageElement? Why is this function named just “path” when the other function is named “computePath”? > Source/WebCore/rendering/RenderImage.cpp:494 > UNUSED_PARAM(paintInfo); Need UNUSED_PARAM for paintOffset too to keep iOS building. > Source/WebCore/rendering/RenderImage.cpp:510 > + const auto* areaElementStyle = areaElement.computedStyle(); Why the const here? Needed? > Source/WebCore/rendering/RenderImage.cpp:533 > + paintInfo.context().drawFocusRing(path, document().page()->focusController().timeSinceFocusWasSet(), needsRepaint); This overload of drawFocusRing is defined for PLATFORM(MAC) only, but this is platform independent code. What guarantees that document().page() is not null?
Created attachment 276017 [details] Patch
Comment on attachment 276017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276017&action=review > Source/WebCore/html/HTMLAreaElement.h:28 > +#include "Path.h" Why do you need this include here? > Source/WebCore/rendering/RenderImage.cpp:543 > +#if PLATFORM(MAC) > + bool needsRepaint; > + paintInfo.context().drawFocusRing(path, document().page()->focusController().timeSinceFocusWasSet(), needsRepaint); > + if (needsRepaint) > + document().page()->focusController().setFocusedElementNeedsRepaint(); > +#else > + paintInfo.context().drawFocusRing(path, outlineWidth, areaElementStyle->outlineOffset(), areaElementStyle->visitedDependentColor(CSSPropertyOutlineColor)); > +#endif Maybe we can clean this up in a later patch. > LayoutTests/fast/images/image-map-outline-in-positioned-container.html:24 > + if (!window.eventSender) > + return; > + eventSender.mouseMoveTo(35, 35); > + eventSender.mouseDown(); > + eventSender.mouseUp(); Maybe just use foo.focus().
Created attachment 276019 [details] Patch
Comment on attachment 276019 [details] Patch Attachment 276019 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1121869 New failing tests: fast/images/image-map-outline-with-scale-transform.html
Created attachment 276025 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.10.5
Created attachment 276028 [details] Patch
Comment on attachment 276028 [details] Patch Clearing flags on attachment: 276028 Committed r199247: <http://trac.webkit.org/changeset/199247>
All reviewed patches have been landed. Closing bug.
Thanks! https://github.com/twbs/bootstrap/commit/62c07b67bffd4af51de354299eac01d905323c03