display: block for details/summary hides drop arrows in Firefox (normalize.css update)

Just wanted to confirm I'm seeing a difference indeed when using Firefox (54.0 - Ubuntu):

And with Chrome (59.0.3071.104 - Ubuntu):

Working on triaging this issue at Drupalcon Vienna. I follow the instructions in issue #2474049. @ekl1773 is mentoring.

I've tested the summary element in different browsers and can reproduce it. Only in Firefox the arrow next to the summary is missing. The Version of Normalize currently used is 3.0.3. The error is fixed in version 5.0. See this issue for further information: https://github.com/necolas/normalize.css/issues/640. So the idea would be to update to a newer version of normalize.css to fix this error.

Updated Normalize.css Version to 7.0.0. This fixes the error.

reviewing.

I can confirm that the details element is now properly formatted in Firefox:

Before the patch:

After the patch:

However, since the update of Normalize.css changes a bunch of other elements too, I also browsed around the rest of the interface, to make sure nothing else substantial has been changed with the update of the Normalize library. Front end pages appear completely unchanged, and every administrative interface I checked (node/add, user/1/edit, admin/content, admin/config, admin/modules, admin/reports/status, admin/structure/types/manage/article/fields, admin/config/media/image-styles/manage/thumbnail) looked fine as well.

Possibly also of use: we also included the latest version of Normalize.css into Backdrop CMS v 1.8.0 on Sept 15th, and have seen no adverse repercussions for updating the library: https://github.com/backdrop/backdrop-issues/issues/2602

Thanks @Eric Heydrich and @jenlampton for working on this! Adding credit for the triage.

It looks like this library was last updated before release. Since this updates a vendor library several major versions, it needs to wait for 8.5.x.

The library is loaded as part of Classy, so it would count as a BC break for one of the stable base themes and we need to look at the impact vs. disruption. The alternative might be to add a more targeted fix overriding the CSS in Classy -- not sure. So +1 on leaving this one for a frontend framework manager. Leaving RTBC for their review.

Thanks!

@Cottser, @effulgentsia, and I discussed this issue today. Since this library is used in Classy, we're not sure we should pull the trigger on updating the whole library just yet. However, we also don't want to fork the existing (old) version of the library. And this is a clear bug that causes noticeable usability and possibly accessibility issues, so the impact is significant and probably even worth backporting a fix.

So, what we recommend is to add an additional stylesheet to the normalize library that overrides the problematic rule, maybe with a change record since it'd still be changing the stable base themes. Then, we can explore whether to go the rest of the way for updating normalize.css in #2919117: [policy, no patch] Discuss how to update 3rd party CSS libraries while still proving BC for stable themes, and look at that for a future minor release.

Thanks @jhodgdon, @jenlampton, and @Eric Heydrich!

Lets fix this for 8.6 :)

Working fine on Mac with Safari

Updating the browsers where the patch was tested.

I have a Linux machine with Firefox, and the patch worked the same way as reported above.

I also have a VM with Windows and tested the patch with IE 11 and Edge.
On both of them, the site information page looked correct before the patch. After I applied the patch, there was a small dot on the page next to each field group.
I tried putting a list-style: none; in the new css file next to the other line, and that fixed the issue on IE 11 and Edge, but it removed the triangle again on Firefox.

I tested the patch from #6, and it had the same effect on the collapsible field groups in IE 11 and Edge.

We might have to do something to selectively apply styles in IE and Edge, but I don't know how to do that.

Updating my attribution. Somehow a bunch of issues I commented on recently have acquired a customer. Didn't intend that.

Crediting everyone who worked on the duplicate issue #2873390: No arrow on details blocks with Firefox

Ok, so I tested patch in #19 and before applying that I was having the issue only in firefox, chrome and safari were fine. After applying that patch all the browsers were fine. Also in the extend menu and then module description, I couldn't see the drop-down arrows before in Firefox, now I'm able to see those too, I'm using Mac.
Chrome:

Firefox:

Safari:

Before adding the patch #19 my extend section of firefox and site-info section was as follows:

the extend section after patch:

I'd suggest the following to scope the patch to Firefox:

@-moz-document url-prefix() { 
    summary {
       display: list-item;
    }
}

I understand 'display: list-item' can cause problems for IE/Edge, which is why we should only target Firefox.

Re rolled patch targeting firefox only.

I am adding the "Needs manual testing" tag in order to make this issue easier to find.

I've tested #51 on my local and also did the manual testing on Windows and Mac. Tested on all browsers and it looks fine to me. Adding screenshots of all the browsers I tested on.

+1 to commit the patch from #51, which corrects the missing triangle in Firefox. That's the main usability bug here, and I think it would be good to limit the scope of this issue to fixing the triangle.

What the patch in #51 doesn't do is address the part about the summary being blue, or black. The screenshots in #54 show that IE and Edge have a blue summary text, while Firefox, Chrome, Safari, and Opera all have black summary text. This is because IE and Edge are the only browsers that haven't implemented details/summary behaviour yet, so they get the collapse.js polyfill. The blue colour comes from this polyfill, which adds an <a> element inside the summary. The summary text therefore picks up the blue colour of links in general. Firefox gained support for details/summary in version 49, and prior to that it used the collapse.js polyfill, and got the blue link colour.

Chrome, Opera, and Safari have supported details/summary for much longer, and haven't been using the polyfill since before D8.0.0 was released. The old styleguide proposal for Seven shows blue text for the summary, but these browsers have had black summary text ever since D8 was released.

Given that (a) Chrome is the most-used browser, and has shown black summary text since D8.0.0, and (b) there's a plan to modernize Seven in #2957457: Proposal: Modernize the look/feel of the Seven admin theme, can we say that resolving this blue vs. black issue doesn't have to be dealt with here? We could have a follow-up issue to make the colour consistent, which ever one we want (IE/Edge being the odd one out).

Fixing the arrows is something we can benefit from straight away, and might be cherry-picked back to D8.5.x.

--- /dev/null
+++ b/core/assets/vendor/normalize-css/normalize-fixes.css

The thing that is confusing here is that this is in the vendor space but it is not vendor code. It's our code. A problem is that I'm not sure where else we could put it.

Here's a update which puts the normalize fixes in core/misc

A firefox screenshot to confirm the fix works in Bartik too.

core/misc - nice dumping ground! Mich better than core/assets/vendor

re #61 - Good point. Is a @see tag right for this? Can't roll a patch just now, reading issues on a phone.

Fixes #61, changes only to docblock.

Very nice that the approach taken on the patch targets only Firefox which makes testing much easier. I tested this manually and made sure that the CSS file gets loaded in the right place. Committed 37059c8 and pushed to 8.6.x. Thanks!

Hooray, thanks everybody for seeing this through!

Now what to do with update & maintenance of the lib #2821525: Update normalize.css to the most recent version

Can this be cherry-picked to 8.5.x please? I don't know how we mark an issue for that.

It's a UX bug fix we can benefit from straight away.

Queued an 8.5.x test for patch #65.

Could it be that this issue is back, but now on chrome as well - or it's just me? I am seeing this on a D8.9.13 site with Seven.

@hanoii - Did you end up creating a new issue? This is definitely happening again.

I think I'm seeing this as well in latest D9, we should really get a new ticket filed.

Added missing mentoring credit.