Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
When you have multiple upload fields, and click the upload button, it uploads the file and disables the other upload fields as expected, however the upload fields stay disabled and you need to click the upload button to re-enable them. See attached image.
Comment | File | Size | Author |
---|---|---|---|
#23 | d7-use_attr_with_false-1480568-23.patch | 1.07 KB | gagarine |
#8 | d7-use_attr_with_false-1480568-8.patch | 1.09 KB | gagarine |
#10 | d7-use_attr_with_false-1480568-8.patch | 1.52 KB | gagarine |
#6 | d7-use_.attr_with_false-1480568-6.patch | 1.07 KB | gagarine |
#6 | d8-use_.attr_with_false-1480568-6.patch | 569 bytes | gagarine |
Comments
Comment #1
henwan CreditAttribution: henwan commentedAfter looking at the core file/file.js code, I noticed that it is using this to re-enable fields:
$fieldsToTemporarilyDisable.attr('disabled', '');
That will still keep the fields disabled. You will need to remove the disabled attribute in order to re-enable the field:
$fieldsToTemporarilyDisable.removeAttr('disabled');
I've attached a patch for it.
Comment #2
Mark Theunissen CreditAttribution: Mark Theunissen commentedSeems like a sensible change.
Comment #3
gagarine CreditAttribution: gagarine commentedI agree and tested #1. Works for me.
We can also use attr but set to false
But, I'm for pushing this patch, it's better to remove the attribute because it's what we want to do...
I think this is linked to jqueryUpdate and/or HTML5 theme.
FYI they are some change in jQuery 1.7 than made bad code incompatible. #1498858: [meta] attr and prop.
I put as RTBC because it look predictable for me. But feel free to ask more test if you feel uncomfortable.
Comment #4
marcingy CreditAttribution: marcingy commentedThis needs to be fixed in d8 first and then backported.
Comment #5
gagarine CreditAttribution: gagarine commentedI create also a slightly different issue for D8 as we can use prop #1597038: Use .prop instead of .attr when we deal with properties
After looking around I saw some browser inconsistently with .removeAttr on some kind of attribute/properties. So at the end it will be safer to use attr('disabled', false);.
We should do a quick search in all core's javascript files and replace .attr(myAttribut, '') by .attr(myAttribut, false). I can do it if you agree on the idea.. The patch shouldn't be that big.
Comment #6
gagarine CreditAttribution: gagarine commentedI found only two place where we have this problem on D7 and one on D8.
regex used
Comment #7
ericduran CreditAttribution: ericduran commentedDon't really have much to say. Patch is pretty straight forward.
Comment #8
gagarine CreditAttribution: gagarine commentedI new patch for D7 as it didn't past the test. I also use full if syntax because I think short if syntax is not really clear in this context.
EDIT: Ah I understand the system try to apply the patch again 8.x only... so both patch works.
EDIT 2: don't use this one.. look at #10
Comment #10
gagarine CreditAttribution: gagarine commentedOups I forget to edit the second file in #8.
Comment #12
gagarine CreditAttribution: gagarine commentedIt fail because it's for D7. D8 patch already passed test in #6.
Comment #13
nod_tag please.
Comment #14
gagarine CreditAttribution: gagarine commentedTo test it, patch (#6 for D8, #10 for D7) and check this:
- On the page admin/structure/types/manage/YOUNODETYPE/fields add a field, move a field, save..
- On a node with two file fields upload a file (ajax upload) on the first field, upload a file on the second field, save the node.
Comment #15
ryan.gibson CreditAttribution: ryan.gibson commentedThe patch applied cleanly, I reviewed the code and it is not complicated and does what it says, I also completed the tests from #14 and everything works fine after applying the patch.
Comment #16
JamesK CreditAttribution: JamesK commentedWhy don't you leave the condition syntax the way it was?
The line should become
$(this).html(html).attr('disabled', disabled ? 'disabled' : false);
Comment #17
JamesK CreditAttribution: JamesK commentedSorry my problem is with D7 patch, D8 is still RTBC.
Comment #18
gagarine CreditAttribution: gagarine commented@JamesK I actually wondering myself about this point. So I read http://drupal.org/node/172169 and so nothing about short syntax... and I suppose it was not encouraged ( http://drupal.org/node/172169#comment-6044038) . I have to say personally I never liked short syntax.
But once the D8 get committed I can provide a patch with the short syntax if needed.
Comment #19
MustangGB CreditAttribution: MustangGB commentedAfter some chitchat on IRC the general consensus is that ternary operators are fine to use, and in this case should be, as long as we're sensible about it. e.g. 80 character per line rule
So lets go with #16 and get a D7 patch rolled as well.
Comment #20
sunComment #21
catchThanks. Committed/pushed to 8.x.
Comment #22
MustangGB CreditAttribution: MustangGB commentedAs per #16 a reroll in needed for D7
Comment #23
gagarine CreditAttribution: gagarine commentedreroll
Comment #24
JamesK CreditAttribution: JamesK commentedLooks great.
Comment #25
Macronomicus CreditAttribution: Macronomicus commentedNice to have this sorted, tested perfect for me.
Cheers!
Comment #26
MustangGB CreditAttribution: MustangGB commented#23 Has been working fine over here also.
Comment #27
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/74de092