WordPress.org

Make WordPress Core

#43082 closed defect (bug) (fixed)

Add plugins search results: the plugin details modal opens in the thickbox modal

Reported by: afercia Owned by: afercia
Milestone: 4.9.2 Priority: normal
Severity: normal Version: 4.9
Component: Plugins Keywords: has-screenshots has-patch fixed-major
Focuses: accessibility, javascript Cc:
PR Number:

Description

To reproduce:

  • go in Plugins > Add New
  • open a plugin details modal, for example from the "Featured" tab, click on the BuddyPress "More details" link
  • the modal opens in the custom WordPress modal:

https://cldup.com/0c9aYWMwlg.jpg

  • now go in the Search tab and search for "buddypress"
  • click on the BuddyPress "More details" link
  • the modal opens in the default thickbox modal:

https://cldup.com/E-KqeYJjtY.jpg

Worth noting the default thickbox modal doesn't handle well some accessibility requirements, for example tabbing is not constrained within the modal and Escape to close the modal works only in the modal top toolbar.

Seems to me this worked correctly in 4.8 and was introduced at some point in 4.9.

Attachments (2)

43082.diff (831 bytes) - added by afercia 22 months ago.
43082.2.diff (1001 bytes) - added by afercia 22 months ago.

Download all attachments as: .zip

Change History (13)

#1 @afercia
22 months ago

  • Milestone changed from Awaiting Review to 4.9.2

Seems to me this is a consequence of [41356]. The "custom modal" (which basically is some JS and CSS overriding) now kicks in only for modals with a plugin-details-modal CSS class, which in turn gets added only when clicking on the "More Details" link.
Worth noting the click event attached to the More Details link hasn't really ever worked in the Search results because the AJAX search rebuilds the HTML. Instead of attaching the event directly on the links, the event needs to be delegated.

@afercia
22 months ago

#2 @afercia
22 months ago

  • Keywords has-patch added
  • Owner set to afercia
  • Status changed from new to assigned

#3 @afercia
22 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 42443:

Plugins: Fix the plugin details modal in the install plugin search tab after [41356].

Fixes #43082

#4 @afercia
22 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.9.2 consideration.

#5 @SergeyBiryukov
22 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 42456:

Plugins: Fix the plugin details modal in the install plugin search tab after [41356].

Props afercia.
Merges [42443] to the 4.9 branch.
Fixes #43082.

This ticket was mentioned in Slack in #core by afercia. View the logs.


22 months ago

#7 @afercia
22 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

As reported by @rinkuyadav999 on Slack this broke the modal "Close" button. Quoting from the jQuery documentation:

https://api.jquery.com/event.stoppropagation/

Since the .live() method handles events once they have propagated to the top of the document, it is not possible to stop propagation of live events. Similarly, events handled by .delegate() will propagate to the elements to which they are delegated; event handlers bound on any elements below it in the DOM tree will already have been executed by the time the delegated event handler is called. These handlers, therefore, may prevent the delegated handler from triggering by calling event.stopPropagation() or returning false.

Thus, this event still needs to be delegated but preferably on the closest available ancestor, to avoid conflicts with other handlers.

@afercia
22 months ago

#8 @afercia
22 months ago

  • Keywords fixed-major removed

43082.2.diff delegates the event on the closest ancestor. To test:

  • before applying the patch, open a plugin details modal
  • click on the X "close" button on the top right
  • verify the modal doesn't close
  • apply the patch
  • repeat the above steps
  • verify the modal closes correctly

#9 @afercia
22 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 42491:

Plugins: Fix the plugin details modal "Close" button after [42443].

Props rinkuyadav999.
Fixes #43082.

#10 @afercia
22 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening (again) for 4.9.2 consideration :)

#11 @SergeyBiryukov
22 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 42493:

Plugins: Fix the plugin details modal "Close" button after [42443].

Props rinkuyadav999.
Merges [42491] to the 4.9 branch.
Fixes #43082.

Note: See TracTickets for help on using tickets.