Opened 8 years ago
Closed 8 years ago
#36533 closed defect (bug) (fixed)
Doesn't work browse media libary on Frontend
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.5.3 | Priority: | normal |
Severity: | normal | Version: | 4.5 |
Component: | Media | Keywords: | has-patch fixed-major |
Focuses: | javascript | Cc: |
Description
I'm using browse media library dialog in frontend.
It was working on wordpress 4.4 and previous all version.
I don't understand that issue, It's only work on firefox but it doesn't work on chrome and safari.
Please see the attached images.
Attachments (6)
Change History (50)
#2
@
8 years ago
Hi,
I guess this could be the same problem as described in here: https://wordpress.org/support/topic/wp45-jquery-update-and-table-styles-issue
Regards,
Oliver
#3
@
8 years ago
I have tested this and can confirm @webmandesign's findings that the following CSS breaks the media library...
table { border-style: solid; border-collapse: collapse; }
This is actually not limited to the front-end. It will break in wp-admin as well if that CSS is present.
I've tried targeting media library divs and setting the border-collapse to initial, but the divs don't appear to have any classes or an ID in their initial state, so they can't be targeted.
#4
@
8 years ago
I did some digging and the issue appears to be with the following code starting on line '6764' of 'wp-includes/js/media-views.js`...
if ( $el.is(':visible') ) { return this; }
That is in the wp.media.open
function and is returning true even when the element isn't visible. I'll do some more digging and see if I can find a solution.
#5
@
8 years ago
I did some more testing and think I have figured out the issue as well as a fix.
I'm not sure why, but in the latest version of jQuery, when this CSS is present...
table { border-style: solid; border-collapse: collapse; }
$el.is(':visible')
will return true, even if the element isn't in the DOM.
This is preventing the media library from being shown because wp.media.open
will return on line 6765
of wp-includes/js/media-views.js
even though $el
isn't visible because it hasn't been inserted into the DOM when open
is first called...
if ( $el.is(':visible') ) { return this; }
The solution is to check and see if $el
is in the document in addition to the $el.is(':visible')
check...
if ( $el.is(':visible') && $.contains( document, $el[ 0 ] ) ) { return this; }
I have tested and can confirm that this works to solve the issue. Patch to follow...
#8
@
8 years ago
@justinbusa Thank you so much, you saved my life :). I can't wait for wp updated stable version. Again thank you.
#9
follow-up:
↓ 10
@
8 years ago
I have also an issue with jQuery project, just in case.
@purevtsooj Have you tested the fix? Does it work for your issue as well?
#10
in reply to:
↑ 9
@
8 years ago
Replying to webmandesign:
I have also an issue with jQuery project, just in case.
@purevtsooj Have you tested the fix? Does it work for your issue as well?
Yes, I've just tested the fix, It works fine,
#11
@
8 years ago
Possibly related: https://github.com/jquery/jquery/issues/3043
#12
@
8 years ago
I can also confirm that the fix provided by @justinbusa fixes my issues with opening the Media Modal from the front-end. The border-collapse:collapse directive is included in the normalize.css library, that is bundled with my theme as well as the Zurb Foundation (v5.5) framework.
#13
@
8 years ago
Hi, was wondering if someone could help me out with where to modify this code exactly as I'm not a programmer by default, looking at @justinbusa comment
I did some more testing and think I have figured out the issue as well as a fix.
I'm not sure why, but in the latest version of jQuery, when this CSS is present...
table { border-style: solid; border-collapse: collapse; }
$el.is(':visible')
will return true, even if the element isn't in the DOM.
This is preventing the media library from being shown because
wp.media.open
will return on line6765
ofwp-includes/js/media-views.js
even though$el
isn't visible because it hasn't been inserted into the DOM whenopen
is first called...
if ( $el.is(':visible') ) { return this; }The solution is to check and see if
$el
is in the document in addition to the$el.is(':visible')
check...
if ( $el.is(':visible') && $.contains( document, $el[ 0 ] ) ) { return this; }I have tested and can confirm that this works to solve the issue. Patch to follow...
ths seem to fix the issue I'm just not sure where and what to edit
Thanks,
#14
follow-ups:
↓ 15
↓ 18
@
8 years ago
- Keywords reporter-feedback removed
- Milestone changed from Awaiting Review to 4.5.1
Moving to the 4.5.1 milestone for consideration. media-views.diff looks like a simple fix but needs a simple test case for testing.
#15
in reply to:
↑ 14
@
8 years ago
Replying to ocean90:
Moving to the 4.5.1 milestone for consideration. media-views.diff looks like a simple fix but needs a simple test case for testing.
Sounds good. Good luck to everyone.
#16
@
8 years ago
@pcnetworkx
If you would like to apply the fix, just open the YOUR_WORDPRESS_FOLDER/wp-includes/js/media-views.js
file (in any simple text editor or programmers editor such as Sublime Text) and edit the line number `6764` the same way Justin did. Save the file.
Then copy the whole file content and paste it into the YOUR_WORDPRESS_FOLDER/wp-includes/js/media-views.min.js
file replacing the file's original content. Again, don't forget to save the file ;)
#17
@
8 years ago
looks like a simple fix but needs a simple test case for testing.
@ocean90 great! Would that be a test using QUnit? I'm not familiar with writing tests for WordPress, but if you point me in the right direction, I'm sure I can figure it out. Thanks!
#18
in reply to:
↑ 14
@
8 years ago
Replying to ocean90:
Moving to the 4.5.1 milestone for consideration. media-views.diff looks like a simple fix but needs a simple test case for testing.
From what I've read above, having
table { border-style: solid; border-collapse: collapse; }
in your theme stylesheet is enough to break the media modal. That's when current jQuery falsely reports the table to be visible even though it isn't. See http://jsbin.com/hopipejuve/1/edit?html,js,console, which I also posted on the jQuery issue on GitHub mentioned above.
#19
@
8 years ago
The jQuery team is working on a fix for a similar issue. It's scheduled for 1.12.4, which doesn't have a date yet. There are probably a few other areas with unexpected behaviour because of that bug. Therefore, it would be great if this bug could be resolved by simply updating jQuery.
#20
@
8 years ago
I guess that would be the best fix for the issue. Just updating the jQuery. However, not sure when this happens...
Would be great if Justin's fix get to 4.5.1 update meanwhile as this is issue potentially affecting many users.
This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.
8 years ago
#23
@
8 years ago
- Milestone changed from 4.5.1 to 4.5.2
Unfortunately we don't have a working fix here ready in time for 4.5.1, punting for consideration in 4.5.2.
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
8 years ago
#25
@
8 years ago
I came across this issue while following up on some support requests for a plugin of mine.
It's worth noting that the CSS in question (see swissspidy's comment) is present in Twenty Thirteen, which may make this a more serious regression.
media-views.diff does fix the problem in my specific test case, but I'll leave it to others to judge whether it's worth patching WP to cover a jQuery regression like this one.
#26
@
8 years ago
I just ran into this issue again with another theme, but this time, the following CSS was the culprit...
table { min-height:200px; }
It would be cool if we could get the fix in 4.5.2 if jQuery doesn't get this fixed soon, but I understand not wanting to patch core for an issue related to a third party library.
#27
@
8 years ago
Well, doesn't seem like jQuery update is around the corner now.
So how about patching the issue by downgrading jQuery to v1.11.3 shipped with WordPress?
Better than having this misterious problem ruining experience with WordPress, I think. Or is there some security concern about downgrading or something?
This ticket was mentioned in Slack in #core-images by mike. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by joemcgill. View the logs.
8 years ago
#31
@
8 years ago
- Owner set to adamsilverstein
- Status changed from new to assigned
Tested this issue and reproduced by adding the table class and installing the wp front-end editor plugin. Before the patch, the media modal fails to open, with the patch the issue is fixed.
The code in question is used when opening the modal to check if its already open and return it if so. The jQuery bug breaks this line and adding the additional check fixes that be ensuring the modal only returns the expected element if it is actually present in the DOM.
Although this fix works, I think the upstream fix is preferable to this check if it available in time for including in a point release.
#32
follow-up:
↓ 33
@
8 years ago
- Keywords needs-testing added
Good news!
Apparently this has been fixed upstream in 1.12-stable, which means jQuery 1.12.4 is just around the corner.
Here's what changed since 1.12.3: https://github.com/jquery/jquery/compare/1.12.3...1.12-stable
It would be great if someone could test the current version from GitHub to verify the fixes.
#33
in reply to:
↑ 32
@
8 years ago
Replying to swissspidy:
It would be great if someone could test the current version from GitHub to verify the fixes.
36533.patch includes the version of the current 1.12-stable branch. I propose to commit the patch to trunk for further testing.
#34
@
8 years ago
I have tested this with 36533.patch jQuery version and it works fine for me on Windows Chrome, Opera, IE11, FireFox. No issue.
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.
8 years ago
#41
@
8 years ago
jQuery 1.12.4 has been released 3 days ago, see https://blog.jquery.com/2016/05/20/jquery-1-12-4-and-2-2-4-released/
We fixed a sticky issue for those using the AMD source and a “:visible” selector bug in 1.12.3.
@ocean90 Mind updating jQuery in trunk to this stable release?
Hi @purevtsooj, thank you for the bug report.
Its difficult to tell from your screen shot what may be happening. This may be a core issue, or it might be something about how you have coded the front end media library.
Do you see any errors in your browser debugger console?
Can you share a more complete code snippet or gist of the code so I can reproduce this issue locally?