I was working on #514990-86: Add a UI for browsing tokens today and someone suggested I create a patch that uses the new fieldset-replacing '#type' => 'details' element for the token browser.

So I did. But although using it worked fine in the version of Chrome I have installed currently, my usual/default browser (which is currently Firefox 13 on Ubuntu) did not have such good luck. The elements look just the same on the two browsers, but clicking them has no effect in Firefox.

I think all elements are supposed to degrade to something useful in older browsers, right? This one apparently doesn't at this time, at least with the patch on comment #86 on that issue (it's possible I did something wrong but I don't think so). See the token.pages.inc component of the patch if you're interested.

CommentFileSizeAuthor
#6 drupal8.dblog-filter.6.patch1.21 KBsun
#5 baddetailsformatting.png29.17 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rooby’s picture

rooby CreditAttribution: rooby commented

One question I have is do we need to even support firefox 13?

Don't we only support the latest of most browsers (specifically the ones that now auto update) except IE?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Title: New 'details' element does not work in Firefox 13 » New 'details' element does not work in most recent versions of Firefox, Opera, or IE

See http://caniuse.com/details, only Safari 6.0 and Chrome 12+ support it.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

Already raised here #1839158: Replace collapse.js with a proper polyfill for <details>

ry5n’s picture

ry5n CreditAttribution: ry5n commented

While we wait on the polyfill issue that nod_ mentions, the normal collapse.js behaviour should still be taking over if <details> is not natively supported. In my own testing for the original patch, I can confirm that this works in FF 16, Opera 12.11, Safari 6 (all on Mac) and IE 9. There are known issues currently with IE8 that should be fixed along with a proper polyfill.

However, I'd like to fix this if it's not too hard, pending that issue. @jhodgdon, are you seeing errors in the JS console? You mentioned Ubuntu, I wonder if this is a platform-specific issue with the modified collapse.js? Any other info you can provide would be useful.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
FileSize
baddetailsformatting.png29.17 KB

Hm.

I had to reinstall d8 today (my site was totally unusable due to some updates apparently)... So I went to admin/reports/dblog (which uses a 'details' element) and it is ... kind of working. On this page, I get no JS errors, but when I expand the "filter log messages" details element, the formatting is really bad (see screenshot -- note that the details element before that says "clear log messages" has been moved to the inside after expanding).

Then I applied the patch in the other issue, enabled the new Token module, cleared the cache, and went to admin/config/people/accounts, and opened the token browser dialog. [And by the way, on the Modules list page, the details/fieldset things seem to be working OK for me.]

On this dialog, the 'details' elements are not working. There are no JS errors... I think the reason is that the dialog pop-up is not triggering the JS that should be modifying those elements so they will work. Do I need to do something special to trigger this?

I also changed the patch so that the token browser opened in a new window, and the fieldsets are not working there either (they are just showing up expanded). That window is not getting the JS added to it, which is probably due to my patch and not relevant for this issue.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: New 'details' element does not work in most recent versions of Firefox, Opera, or IE » "Filter log messages" details on Recent log messages page applies bogus floats
Component: markup » dblog.module
Status: Active » Needs review
FileSize
drupal8.dblog-filter.6.patch1.21 KB

Unless I'm missing something, then the originally reported issue is obsolete, or rather, an entirely different issue, since #attached JavaScript (and thus collapse.js) does not seem get loaded for the Token UI Dialog code in #514990: Add a UI for browsing tokens for some reason, but that's unrelated to the new details.

Therefore it seems like all that remains for this issue is the Dblog UI bug, as visible in #5. Thus, reclassifying this issue.

Attached patch fixes the custom dblog.css that is applied there.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Thanks, I agree with the assessment in #6. Sorry for not updating the issue properly!

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Reviewed & tested by the community

In seven styles.css .form-item takes over the margins specified by dblog.css getting rid of the margin-right. It's fine in bartik and stark.

The two selects are a little close to each other in seven but then again, not a big deal, the bug is fixed.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Did someone test this on an older browser (like Firefox 13 as reported here) and verify that it works?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Two reasons for why FF 13 doesn't have to be tested:

1) Our current browser support policies only include the current version and current - 1 (which is 16 as of today).

2) The new CSS in the patch does not use any special/fancy CSS to begin with, so this should technically work on Firefox 3. ;)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

OK, well at least testing on a browser that exhibited the reported bug would be useful? :)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

I'm working and developing in Firefox, so that's totally covered :)

I also forgot to mention that I only updated to latest FF yesterday. Which means that I developed and tested the patch in #6 on FF 15. ;)

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.