Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
file.module
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
9 Jan 2013 at 11:52 UTC
Updated:
29 Jul 2014 at 21:44 UTC
Jump to comment: Most recent file
Comments
Comment #1
yesct commentedI might have the component wrong...
Here is a screenshot.

Comment #2
yesct commentednext steps (in any order):
Comment #3
swentel commentedDon't see any problems here ..
Comment #4
swentel commentedComment #5
yesct commentedI double checked again. did a sudo rm -r sites; git checkout sites; git checkout 8.x; git pull --rebase; drush -y si ...
the screenshot in was in Firefox 17.0.1
Can still reproduce that
In Chrome
I get something different, almost the same, but with a arrow showing the field is collapsed.
Here is a screenshot of Chrome (Version 23.0.1271.101)

It expands ok, but after adding each image it "jumps" back to collapsed.
Comment #6
swentel commentedSo that's a DETAILS problem (the new tag instead of fieldset - see #1168246: Freedom For Fieldsets! Long Live The DETAILS.), not a field problem. Could be that this version of firefox has a bug maybe ..
Comment #7
sunThere are two bugs here:
1) Details are always collapsible. #collapsible is obsolete. The rendering should no longer account for a potential FALSE value. (The remaining clean-up will happen in #1852104: #type details: Remove #collapsible property)
2) File module implements a custom multiple field widget behavior and manually implants theme_details() as a #theme_wrappers — that's completely missing all other element type properties of #type 'details', which means that collapse.js doesn't get loaded at all.
Comment #9
swentel commented#7: drupal8.file-multiple-details.7.patch queued for re-testing.
Comment #10
swentel commentedThat indeed fixes it, and I was wondering about that #collapsible thing, good to know about the other issue :)
If this comes green and YesCT confirms, this is RTBC from me.
Comment #11
yesct commentedI've delayed, but I can verify that it fixes it in both firefox and chrome.
it also still applies.
Comment #12
yesct commented#7: drupal8.file-multiple-details.7.patch queued for re-testing.
Comment #14
yesct commented#7: drupal8.file-multiple-details.7.patch queued for re-testing.
Comment #16
yesct commentedtagging for reroll (reroll doc: http://drupal.org/patch/reroll)
Comment #17
swentel commentedRerolled after the #collapsible cleanup happened.
Comment #18
yesct commentedThis might not need tests since it was browser dependent.
If someone knows how tests should be done, please provide some details to help direct that.
(untagging needs tests and also needs reroll since that was done. thanks. :) )
Comment #19
catchWas this really browser dependent? I'd expect the missing #type => details to have an impact on the HTML so presumably we could test for the HTML with it and that'd fail without?
Comment #20
sunYes, this bug depends on the browser being used, due to native browser support differences for details, which circles back into #1839158: Replace collapse.js with a proper polyfill for <details> — that is, because the render element manually specifies a #theme_wrappers of 'details', which means that it goes through the appropriate theme function, but alas, just without any element type defaults + pre_render/preprocessing.
This is simply a brain-dead oversight in form declaration. It's like forgetting to fill out the x in 1 + x = 2.
Due to that, I disagree with the need for tests here. Stuff like this just simply happens during refactoring.
Just simply consider the phpDoc for such a test:
Oh, wow. ;)
Comment #21
catchHmm OK if we had browser acceptance tests we could actually test that nicely but agreed that makes no sense with simpletest. Committed/pushed to 8.x.
Comment #23
Gaelan commented