Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#40579 closed defect (bug) (fixed)

Plugin names no longer showing up in core views

Reported by: otto42's profile Otto42 Owned by: melchoyce's profile melchoyce
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch has-screenshots
Focuses: administration Cc:


Just the ellipsis, no names appearing. No idea why. Trunk, Latest Chrome.

Attachments (4)

40579.PNG (235.4 KB) - added by SergeyBiryukov 7 years ago.
40579.diff (2.0 KB) - added by afercia 7 years ago.
40579.2.diff (1.5 KB) - added by mrwweb 7 years ago.
40579.3.diff (2.0 KB) - added by mrwweb 7 years ago.

Download all attachments as: .zip

Change History (32)

7 years ago

#1 @SergeyBiryukov
7 years ago

  • Component changed from General to Plugins
  • Focuses administration added

Can't reproduce here: 40579.PNG.

#2 @Otto42
7 years ago

Hmm. Must be something browser specific then. This was reported to the plugins team email, which was the only reason I noticed it.

Happens in Chrome Version 59.0.3071.16 (Official Build) dev (64-bit) on Windows 10.

#3 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 4.7.5

Confirmed after updating Chrome to 58.0.3029.81 (previously used 57.0.2987.133).

#4 @SimonRWaters
7 years ago

I found it in Chrome Version 58.0.3029.81 (64-bit) for Mac, so would be consistent with a Chrome change, not platform specific. Not reproduced in Firefox.

#5 @afercia
7 years ago

Confirmed after updating Chrome to 58.0.3029.81 (previously used 57.0.2987.133).

Same here. The CSS for the plugin name could benefit from some improvements though.

7 years ago

#6 @afercia
7 years ago

  • Keywords has-patch has-screenshots added

First pass for a patch. Some testing would be nice :)

I'd suggest to test opening the detail modal of a plugin with an insanely long name, for example:

  • test in the desktop and responsive view
  • test other plugins with no banner (or remove the class with-banner using your browser's inspector); for example, some of the Beta Testing plugin have no banner
  • if you want to have fun, using your browser's inspector add contenteditable to the plugin name h2 and edit the name to shorten it or make it longer :)

#7 @afercia
7 years ago

For the records, this is what's happening on Chrome 58.0.3029.81:

#8 @afragen
7 years ago

This was working correctly in MacOS Version 57.0.2987.133 (64-bit), but just had a Chrome update to Version 58.0.3029.81 (64-bit) and it now fails.

Definitely a Chrome bug. Digging into the return and looking at the source show the name is present.

Last edited 7 years ago by afragen (previous) (diff)

#9 @afercia
7 years ago

  • Milestone changed from 4.7.5 to 4.8

I'd say this should be fixed for 4.8. Some testing would be nice.

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

7 years ago

This ticket was mentioned in Slack in #meta by otto42. View the logs.

7 years ago

#12 @Presskopp
7 years ago

This is still in 58.0.3029.110 (64-bit). Unchecking the

#plugin-information-title.with-banner div.vignette {

[ ] width: 772px; uncheck or set to max-width

solves the issue.

#13 @mrwweb
7 years ago

This is a really odd bug. It took me a long time, but I was finally able to work up a reduced text case:

To try to summarize as best I can figure: The .plugin-information-title uses white-space: nowrap; for layout so the floated vignette and title can overlap. It also has overflow: hidden; to "crop" the vignette. However, I think the browser sees that the floated vignette should be "pushing the title out of the way" due to the parent's overflow: hidden and so triggers the text-overflow ellipsis. I'm not even sure this is a bug, though I'll probably report it just in case.

Note in the test case the text-overflow on the plugin name isn't the problem, it's the text-overflow on the parent!

I see in Chrome that the vignette is also too wide when there's a scroll bar on the modal, so refactoring the position similar to what @afercia did seems like the right approach. I noticed a couple spacing things with the first patch, so I'll try to work a v2 up soon.

7 years ago

#14 @mrwweb
7 years ago

Just uploaded my attempt at a patch. It does the following:

  • Only use text-overflow on the title element itself, no need for it on parents
  • Absolutely positions vignette div with correct width
  • Adjust padding/margin/max-width to ensure title is centered when full width and with RTL languages

(As noted above, I'd add testing any path with dir="rtl" to ensure it behave correctly for non-LTR languages.)

#15 @mrwweb
7 years ago

Reported chrome bug may be useful for following:

#16 @afercia
7 years ago

@mrwweb excellent codepen and testing :) A few considerations:

the previous patch was removing also the top right left used for position relative, because as far as I know they don't do absolutely anything when set to 0 :)

position: relative;
top: 0;
right: 0;
left: 0;

I'd consider to remove them.

Couldn't get the math behind max-width: 665px; seems to me theres a bit more space and it should be max-width: 680px;

The previous patch was also fixing the vignette in the responsive view, since seems to me width value of 800% is a bit too much. The vignette extends to the right and the shadow effect is lost, this should be fixed:

There is an edge case when the modal height is very small, the toolbar and the plugin name may overlap the footer because of their z-index. This is a pre-existing issue and I'm not sure it should be fixed in this ticket, or fixed at all.

#17 @mrwweb
7 years ago

Thanks @afercia. I was running into some weird issues when I removed those top/left/right values, but I'll go back in and take a look. I'm trying to change as little as possible since the whole setup feels a little rickety and deserving of a bigger overhaul. That overlap issues feels like a separate bug to me.

I think our math may be coming out differently due to scrollbar issues. Let me see if I can just get rid of the magic number altogether.

#18 @afercia
7 years ago

I think our math may be coming out differently due to scrollbar issues.

Ah right if you're testing on Windows the scrollbar takes some space.

7 years ago

#19 @mrwweb
7 years ago

New patch:

  • Gets rid of that code @afercia ditched in first patch. Should've caught that.
  • Adjust #plugin-information-title padding to match modal content padding.
  • Allows us to use max-height: 100%; for title h2 and avoid need for magic number. (Also avoids inconsistent application of margin between browsers.)

On Windows 10 I've tested Chrome 58.0.3029.96 and Firefox 53.0.2. Both look good to me at narrow- and full-width. More testing would be appreciated.

I see that narrow screens were limiting the title to 85%. I don't know if we want to keep that at large screens instead of 100%.

#20 @afercia
7 years ago

Yep, 85% is also a magic number but I wouldn't be so worried about that :) Will ping in #core to ask for more testing, thanks @mrwweb !

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

7 years ago

This ticket was mentioned in Slack in #forums by otto42. View the logs.

7 years ago

This ticket was mentioned in Slack in #forums by mrwweb. View the logs.

7 years ago

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

7 years ago

#25 @melchoyce
7 years ago

Tested 40579.3.diff and it looks good on Chrome 58.0.3029.96 on multiple screen sizes, and in Firefox 48.0 and Safari 10.1 (12603. too. Would be good to get someone to test on Android and iPhone to confirm.

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

7 years ago

#27 @melchoyce
7 years ago

  • Owner set to melchoyce
  • Resolution set to fixed
  • Status changed from new to closed

In 40672:

Plugins: Fix plugin name display on Chrome 58 and above.

On Chrome 58 and above, plugin names within the plugin details modal were collapsed down into an ellipsis. This fix rewrites the CSS to make sure the titles are always shown.

Props mrwweb, afercia, Presskopp.
Fixes #40579.

#28 @melchoyce
7 years ago

If there's problems on mobile, we can re-open this ticket.

Note: See TracTickets for help on using tickets.