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

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

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 #

Created: March 27, 2015, 11:12 a.m.

Download [raw] [tar.bz2]

		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

Expand All Messages | Collapse All Messages

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.

Expand All Messages | Collapse All Messages