Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 6609231193047040: Noissue - Removed dead code passing coordinates for "Block elements" dialog (Closed)

Created:
March 27, 2015, 11:12 a.m. by Sebastian Noack
Modified:
March 27, 2015, 4:55 p.m.
Reviewers:
kzar, Wladimir Palant
Visibility:
Public.

Description

The "clickhide-show-dialog" message is sent along with the coordinates of the mouse event. However, those coordinates are ignored in clickHide_showDialog() since as far as our change history reaches back. It seems that the original idea was to show the dialog at the position you clicked. However, the logic calculating the absolute position is incorrect anyway, which might be the reason the original author ignored those coordinates but forgot to remove the dead code when initially writing that code. Moreover, this seems like a bad idea to me anyway. What's the point of making sure that the dialog will (partly) overlap the selected element.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -6 lines) Patch
M include.postload.js View 3 chunks +2 lines, -6 lines 0 comments Download

Messages

Total messages: 3
Sebastian Noack
March 27, 2015, 11:19 a.m. (2015-03-27 11:19:43 UTC) #1
kzar
LGTM
March 27, 2015, 12:04 p.m. (2015-03-27 12:04:41 UTC) #2
Wladimir Palant
March 27, 2015, 4:55 p.m. (2015-03-27 16:55:11 UTC) #3
LGTM

Confirmed your investigation - appears to be something we inherited from
AdThwart, doesn't make much sense anyway.

Powered by Google App Engine
This is Rietveld