Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2244333004:
Use document.ElementFromPoint instead of eventsender. (Closed)
DescriptionUse document.ElementFromPoint instead of eventsender.
This patch takes care of TODO to replace
eventsender with document.ElementFromPoint.
BUG=637859
Committed: https://crrev.com/50cc56a7af56811b317d2af42d9f22fc4ffba3e0
Cr-Commit-Position: refs/heads/master@{#412801}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Changed test as per the comments. #
Total comments: 4
Patch Set 3 : Test changes as per comments #Patch Set 4 : Changed as per review comments. #
Total comments: 1
Messages
Total messages: 15 (5 generated)
Description was changed from ========== Use document.ElementFromPoint instead of eventsender. This patch takes care of TODO to replace eventsender with document.ElementFromPoint. BUG=637859 ========== to ========== Use document.ElementFromPoint instead of eventsender. This patch takes care of TODO to replace eventsender with document.ElementFromPoint. BUG=637859 ==========
siva.gunturi@samsung.com changed reviewers: + fs@opera.com, srirama.m@samsung.com
@fs, ptal..
https://codereview.chromium.org/2244333004/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html (right): https://codereview.chromium.org/2244333004/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:18: document.elementFromPoint(x, y).dispatchEvent(mouseEvent); I'd suggest just returning the result of document.elementFromPoint and then compare against 'area'. Then the event listener and 'areaClicked' won't be needed.
@fs, ptal.. https://codereview.chromium.org/2244333004/diff/1/third_party/WebKit/LayoutTe... File third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html (right): https://codereview.chromium.org/2244333004/diff/1/third_party/WebKit/LayoutTe... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:18: document.elementFromPoint(x, y).dispatchEvent(mouseEvent); On 2016/08/17 11:26:22, fs wrote: > I'd suggest just returning the result of document.elementFromPoint and then > compare against 'area'. Then the event listener and 'areaClicked' won't be > needed. Done.
https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html (right): https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:15: return coords == document.elementFromPoint(x, y).coords; i think what fs means is just return document.elementFromPoint(x, y); and then do assert_[equals|not_equals](area, checkPointInArea); May be change function name also to something like getElementFromPoint.
https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html (right): https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:15: return coords == document.elementFromPoint(x, y).coords; On 2016/08/18 at 07:09:44, Srirama wrote: > i think what fs means is just return document.elementFromPoint(x, y); and then do assert_[equals|not_equals](area, checkPointInArea); > May be change function name also to something like getElementFromPoint. Yes, exactly.
ptal.. https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html (right): https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:15: return coords == document.elementFromPoint(x, y).coords; On 2016/08/18 07:09:44, Srirama wrote: > i think what fs means is just return document.elementFromPoint(x, y); and then > do assert_[equals|not_equals](area, checkPointInArea); > May be change function name also to something like getElementFromPoint. Done. https://codereview.chromium.org/2244333004/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:15: return coords == document.elementFromPoint(x, y).coords; On 2016/08/18 07:54:44, fs wrote: > On 2016/08/18 at 07:09:44, Srirama wrote: > > i think what fs means is just return document.elementFromPoint(x, y); and then > do assert_[equals|not_equals](area, checkPointInArea); > > May be change function name also to something like getElementFromPoint. > > Yes, exactly. Done.
lgtm https://codereview.chromium.org/2244333004/diff/60001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html (right): https://codereview.chromium.org/2244333004/diff/60001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/images/imagemap-dynamic-area-updates.html:15: equals ? assert_equals(area, document.elementFromPoint(x, y)) What I had in mind was more like: function checkPointInArea(...) { ... return document.elementFromPoint(x, y); } assert_equals(checkPointInArea(...), area); ... but I guess this works.
The CQ bit was checked by srirama.m@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Use document.ElementFromPoint instead of eventsender. This patch takes care of TODO to replace eventsender with document.ElementFromPoint. BUG=637859 ========== to ========== Use document.ElementFromPoint instead of eventsender. This patch takes care of TODO to replace eventsender with document.ElementFromPoint. BUG=637859 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Use document.ElementFromPoint instead of eventsender. This patch takes care of TODO to replace eventsender with document.ElementFromPoint. BUG=637859 ========== to ========== Use document.ElementFromPoint instead of eventsender. This patch takes care of TODO to replace eventsender with document.ElementFromPoint. BUG=637859 Committed: https://crrev.com/50cc56a7af56811b317d2af42d9f22fc4ffba3e0 Cr-Commit-Position: refs/heads/master@{#412801} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/50cc56a7af56811b317d2af42d9f22fc4ffba3e0 Cr-Commit-Position: refs/heads/master@{#412801} |
