Using extlinks to open external links in a new window, but not add the icon, I noticed the module js still appended the span tags to external link a tags. The icons weren't printing but extra markup was and added visual blank spaces at the end of the link text. After digging through the module I figured out this was because several config variables were being declared as boolean but used as string.
For example "Place an icon next to external links." (var: extlink_class) is a boolean, defaults to 'ext', and if the checkbox on the admin config page is unchecked the value is set to '0' (string 0).
In this patch I separated out some boolean vars into a boolean and a string var. I thought about making extlink_class a boolean and if it's true, passing in "ext" to the JS. But to let people extend the module and add other classes I kept a boolean toggle for extlink icon and a string var for the class name. I added two variables for the invisible text and added them to the admin config form, updated to the D8 class for hiding the text (.visually-hidden), and added some checks in the JS around not executing some functions if extlink icon or mailto icon are turned off. I also created an update module hook to set the former boolean variables to the correct strings based off the current value.
Comment | File | Size | Author |
---|---|---|---|
#14 | type-mismatch-2821803-14.patch | 1.44 KB | elachlan |
| |||
#10 | type-mismatch-2821803-10.patch | 1.71 KB | Bunty Badgujar |
#7 | 2821803-reroll-7.patch | 22.31 KB | akalata |
extlink-rjg.patch | 22.26 KB | rjg | |
Comments
Comment #2
rjg CreditAttribution: rjg commentedComment #3
rjg CreditAttribution: rjg commentedComment #4
trwill CreditAttribution: trwill commentedComment #5
trwill CreditAttribution: trwill commentedPatch applies cleanly - note you will have to run db updates if the module is already installed. This patch does fix the erroneous class being applied (and the whitespace).
However, the patch seems to break the 'open new links in an external window' logic if the 'Place an icon next to external links.' option is not selected. I think this likely is due to the logic removed from old extlink.js:69-83. A new check that replaces this logic (starting at extlink.js:70) looks for the add icon property, and if it exists then apply the target="_blank", but if the 'Place an icon next to external links.' is not selected it never applies the target attr, and external links open in the current window.
Comment #6
rjg CreditAttribution: rjg commented@trwill I still need to fix that glitch. Need to find time soon.
Comment #7
akalata CreditAttribution: akalata commentedreroll of #1 against current dev
Comment #8
akalata CreditAttribution: akalata commentedI tried giving this a shot but don't know enough js.
I renamed 'icon_not_class' to 'exclude_icon_class', and tried to pull out the repeatable code:
...
But I'm not getting elements appended to the external_links or mailto_links arrays.
Comment #9
weseze CreditAttribution: weseze commentedWe are also experiencing this issue, but only on older sites, even after updating them. Clean installs do not have this problem.
I'm unsure why this problem exists or were it is coming from, but the solution seems to be to update the config for the "extlink.settings.extlink_class" to "ext" if it is "0".
Below is an update hook that fixes it:
Comment #10
Bunty Badgujar CreditAttribution: Bunty Badgujar commentedHi,
I just made a very small patch which can fix this issue. No need to change js file.
Comment #11
Bunty Badgujar CreditAttribution: Bunty Badgujar commentedComment #14
elachlan CreditAttribution: elachlan commentedComment #15
elachlan CreditAttribution: elachlan commentedComment #16
elachlan CreditAttribution: elachlan commentedWork will continue in the other issue.