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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

LizZerrG created an issue. See original summary.

Anonymous’s picture

elachlan’s picture

Status: Needs review » Needs work

Hi Lizzerrg, Could you please re-roll the patch on the latest version?

Thanks for your work!

naveenvalecha’s picture

Issue tags: +Novice, +Needs reroll
Berdir’s picture

This 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.

Berdir’s picture

Status: Needs work » Needs review
Issue tags: -Novice, -Needs reroll

Also, 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.

Status: Needs review » Needs work

The last submitted patch, 5: extlink-class-boolean-2879058-5.patch, failed testing. View results

Berdir’s picture

Ok, 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?

elachlan’s picture

elachlan’s picture

Issue tags: +Needs tests

We should probably write a functional test for testing when either the mailto or extlink are disabled.

elachlan’s picture

Status: Needs review » Needs work

The last submitted patch, 11: do_not_call_function_and_remove_display_limitations-2879058-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

elachlan’s picture

  • elachlan committed c588cbf on 8.x-1.x
    Issue #2879058 by elachlan, Berdir, LizZerrG: Do not call...
elachlan’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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