In the latest 8.x-1.x-dev (may 15, 2017) there is a fix for not adding class="0" to external links when the option 'Place an icon next to external links' is not checked. Though this fixes the problem, it is not necessary to call the applyClassAndSpan() function when the class_name is '0'. Therefore this check should happen before the class is called.
Also, the issue that the tag is not appended when an external link has display: block; is fixed by adding this display to the if-statement. Since there are more displays to consider (e.g. display: flex;), it is better to remove this if-statement.
Comment | File | Size | Author |
---|---|---|---|
#13 | do_not_call_function_and_remove_display_limitations-2879058-13.patch | 4.82 KB | elachlan |
| |||
#11 | do_not_call_function_and_remove_display_limitations-2879058-11.patch | 4.82 KB | elachlan |
#9 | do_not_call_function_and_remove_display_limitations-2879058-9.patch | 2.76 KB | elachlan |
| |||
#5 | extlink-class-boolean-2879058-5.patch | 439 bytes | Berdir |
| |||
#2 | do_not_call_function_and_remove_display_limitations-2879058.patch | 7.67 KB | Anonymous (not verified) |
|
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedLizZerrG created an issue. See original summary.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous at One Shoe commentedComment #3
elachlan CreditAttribution: elachlan commentedHi Lizzerrg, Could you please re-roll the patch on the latest version?
Thanks for your work!
Comment #4
naveenvalechaComment #5
BerdirThis is a workaround for a different problem, the extlink class is a checkbox, it shoud have the schema type boolean, not string. Then it will be true/false and the JS will work.
The attached patch fixes that, but this also did some other things to the JS that I can't comment on.
No update function, so users affected by this will have to resave their settings form.
Comment #6
BerdirAlso, I find it extremely weird that extlink provides its own minimized JS. it is really not that big and I'm wondering if this is some left-over from before drupal had aggregated JS. It still doesn't minify, but this is not that big and it's painful for resolving merge conflicts.
Comment #8
BerdirOk, that's not right either, the UI is a checkbox but it also has a return value and the JS is using that as a string. Ignore my patch then.
Since the classname is hardcoded anyway and the config gives the illusion that this is configurable, maybe it would be better to change config and return "ext" to the JS if TRUE and otherwise null or so?
Comment #9
elachlan CreditAttribution: elachlan commentedRe-rolled the patch.
Comment #10
elachlan CreditAttribution: elachlan commentedWe should probably write a functional test for testing when either the mailto or extlink are disabled.
Comment #11
elachlan CreditAttribution: elachlan commentedComment #13
elachlan CreditAttribution: elachlan commentedComment #15
elachlan CreditAttribution: elachlan commented