Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jaiiali created an issue. See original summary.

Wim Leers’s picture

Title: Language descriptions of "CKEditor plugin settings" have a broken link » Broken link in description of CKEditor language plugin
Version: 8.2.1 » 8.2.x-dev
Priority: Normal » Minor
Issue tags: +Quickfix, +Novice, +php-novice

Sigh, apparently the UN is not smart enough to avoid linkrot :/ This definitely used to work at some point!

Anyway, this makes for an excellent novice issue :)

icicleking’s picture

Here is a patch that changes the link.

anavarre’s picture

Make it HTTPS even.

arunkumark’s picture

Status: Active » Needs review
Issue tags: -Novice, -php-novice
FileSize
1.05 KB

As per comment #4 rerolled path with HTTPS for the link of previous patch.

lomasr’s picture

Applied the patch , it worked cleanly . I have a suggestion , as the previous link is not working , may be current link will be changed in future . I think we can include Wikipedia link (https://en.wikipedia.org/wiki/Official_languages_of_the_United_Nations) instead of UN official site link. Added a patch please review. Thanks

Wim Leers’s picture

Hm, I'm not sure why we're wrapping it in a Url::fromUri() call. AFAIK that's not necessary at all. I know it already was there, but let's simplify that while we're touching this code anyway.

Wim Leers’s picture

Status: Needs review » Needs work
anish.a’s picture

Status: Needs work » Needs review
FileSize
1.4 KB

Removed Url::fromUri()

Wim Leers’s picture

Status: Needs review » Needs work

I wanted to RTBC, but now noticed this:

<a href=":url">six official languages of the UN (wikipedia list)</a>.

Why add that (wikipedia list) suffix?

  1. That's not relevant information for the reader.
  2. It's not spelled correctly
  3. It breaks existing translations

Please revert that change. The only thing we should change here, is the URL. The text must stay the same.

lomasr’s picture

as per suggestion in #10 . Made the changes. Adding the patch. Please review Thanks.

lomasr’s picture

Status: Needs work » Needs review
Wim Leers’s picture

Status: Needs review » Needs work
-      '#description' => $this->t('The list of languages to show in the language dropdown. The basic list will only show the <a href=":url">six official languages of the UN</a>. The extended list will show all @count languages that are available in Drupal.', [
…
+      '#description' => $this->t('The list of languages to show in the language dropdown. The basic list will only show the <a href=":url">six official languages of the UN </a>. The extended list will show all @count languages that are available in Drupal.', [

This line is still being changed (a space is still being added). That line should not be changed at all.

So very close! :)

lomasr’s picture

FileSize
591 bytes

Sorry wrong patch added.

lomasr’s picture

lomasr’s picture

Thanks for noticing the space. Resubmitted the right patch . Please review.

lomasr’s picture

Status: Needs work » Needs review
Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Thanks.

  • catch committed 48dbd8f on 8.3.x
    Issue #2820979 by lomasr, icicleking, arunkumark, anish.a, jaiiali, Wim...

  • catch committed 3e63126 on 8.2.x
    Issue #2820979 by lomasr, icicleking, arunkumark, anish.a, jaiiali, Wim...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Fixed an un-used use statement on commit.

Committed/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!

Status: Fixed » Closed (fixed)

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