Incorrect use of Drupal.displace in comment module [#2113087]

Problem/Motivation

In order to access the displace properties we should use Drupal.displace as an object so instead use Drupal.displace().top we should use Drupal.displace.offsets.top.

Proposed resolution

Replace all occurrences of Drupal.displace().top by Drupal.displace.offsets.top.

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Comment	File	Size	Author
	drupal-displace-fix.patch	659 bytes	rteijeiro

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 16 October 2013 at 13:20

Component:	javascript	» comment.module
Status:	Needs review	» Reviewed & tested by the community
Issue tags:		+JavaScript, +quickfix, +Spark

Note: I introduced this apparently incorrect use of Drupal.displace in #1991684: Node history markers (comment & node "new" indicator, "x new comments" links) forces render caching to be per user. My bad!

I did manual testing of this patch, and this indeed continues to work correctly.

Thanks, rteijeiro!

Comment #2

nod_

French

Lille

CreditAttribution: nod_ commented 16 October 2013 at 13:22

Well it's not incorrect per-say, it's just less efficient :)

Comment #3

nod_

French

Lille

CreditAttribution: nod_ commented 16 October 2013 at 13:23

and it'd be nice if we fixed that in a more generic way: #1870006: HTML5 validation with table sticky header is misaligned over the toolbar

Comment #4

Wim Leers

Ghent 🇧🇪🇪🇺

CreditAttribution: Wim Leers commented 16 October 2013 at 13:58

#3: agreed, but the comment "new" indicator and accompanying #new fragment are a special case: the #new fragment is inserted dynamically, since #1991684: Node history markers (comment & node "new" indicator, "x new comments" links) forces render caching to be per user, to not break the render cache. In any case, I support that principle, and threw a review your way :)