Problem/Motivation

ReadSpeaker support has told me that the current script link is for a former product, which should instead be in this format, using a new Region value (example values eu and 1234567):

<script src="https://cdn-eu.readspeaker.com/script/1234567/webReader/webReader.js?pids=wr" type="text/javascript" id="rs_req_Init"></script>

With new Region token it could look like this in the URL field:

//cdn-[open-readspeaker:region].readspeaker.com/script/[open-readspeaker:account-id]/webReader/webReader.js?pids=wr

... where [open-readspeaker:region] is a token from a new Region field, with one of the following values:

  • eu: Europe
  • me: Middle East
  • af: Africa
  • as: Asia
  • oc: Oceania
  • eas: East Asia
  • na: North America
  • sa: South America

The same [open-readspeaker:region] value should then also be used in the href of the listen button, like so:

href="https://app-[open-readspeaker:region].readspeaker.com/cgi-bin/rsent?customerid=[open-readspeaker:account-id]&lang=...

Steps to reproduce

Try the new script path, and see that the design changes.

Updating the script URL Might also fix an error, where the player doesn't work (Curl error: 60) ...

Proposed resolution

  • Add new Region field in form
  • Update script URL
  • Update listen button URL

Remaining tasks

  • Decide whether to make this change in current 1.x branch or (if it's too much of a change) instead do it in #3225300: Plan for Open ReadSpeaker 2.0 release
  • Add new Region field in the form (token: [open-readspeaker:region]), with help text
  • Update the script link
  • Update the listen button link

User interface changes

The new ReadSpeaker listen button, will probably have new id's and classes.

Current (former) ReadSpeaker listener
1.1 current, old listen button

New ReadSpeaker listener, after updating script
1.2 updated listen button

API changes

Data model changes

CommentFileSizeAuthor
#2 open_readspeaker-3322662-webreader_url-2.patch480 bytesjoshahubbers
1.2-updated-readspeaker.png6.62 KBressa
1.1-current-old-readspeaker.png9.96 KBressa

Issue fork open_readspeaker-3322662

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ressa created an issue. See original summary.

joshahubbers’s picture

joshahubbers commented
StatusFileSize
new open_readspeaker-3322662-webreader_url-2.patch480 bytes

This patch changes the default configuration, so on install the webreader is used in stead of the old readspeaker.

bskibinski’s picture

bskibinski
Primary language Dutch
Location Breda, The Netherlands
commented
Status: Active » Reviewed & tested by the community

#2 Patch applies correctly, and on module install the default url is now the modern webreader instead of the old readspeaker.
Patch approved! let's get this into open_readspeakers next version :)

ressa’s picture

ressa
he/him
commented
Version: 8.x-1.x-dev » 2.x-dev
Status: Reviewed & tested by the community » Active

ressa opened merge request !15

ressa’s picture

ressa
he/him
commented
Status: Active » Needs review

Thanks @JoshaHubbers and @bskibinski for getting this moving! I see that @enyug created a patch about the same in #3326461: Allow the cdn in listen button link cms manageable. So with a basis in that great work I have created a MR for 2.x which updates the script and adds a region token. Feel free to review.

Should it be for 1.x in stead, or perhaps that can be done separately?

joshahubbers’s picture

joshahubbers commented

Thank you @ressa. We still use the 1.x version because that is the stable version. If this is also fixed in the 2.x version in issue #3326461: Allow the cdn in listen button link cms manageable, I would propose to stick to the 1.x change in this ticket and handle the 2.x changes in the other?

I just read your comments on the other issue, and agree that it fits this issue to create the compete feature.

Personally I would agree to keep the setting on the main form. This is not a setting that would be different for blocks I think.

joshahubbers’s picture

joshahubbers commented

I reviewed the mr. It looks good. Added some small text improvements. And changed the CDN to a select because that is much more userfriendly in this option in my opinion.

Have not tested it in a real live site yet.

ressa’s picture

ressa
he/him
commented

Thanks for the improvement @JoshaHubbers. I agree it's better as a dropdown, and it works well for me, tested locally.

Perhaps @bskibinski or someone else can review our collaborative MR?

mcaddz made their first commit to this issue’s fork.

mcaddz opened merge request !17

mcaddz’s picture

mcaddz commented

MR 15 is working for me on a test site. All looking good.

@ressa I've added a test for this functionality in MR 17. If you or someone else can review we can mark as RTBC?

acbramley’s picture

acbramley commented

Looking good, nice work on the tests. Just a couple of small changes flagged.

Also in the future @mcaddz - we generally would just use a single MR to contribute to. You can click the Get Push Access button to get access to the fork and then push to the existing branch.

mcaddz’s picture

mcaddz commented
Status: Needs review » Reviewed & tested by the community
ressa’s picture

ressa
he/him
commented
Status: Reviewed & tested by the community » Needs review

Thanks @mcaddz, but after changes to the code, someone else needs to review them :)

acbramley’s picture

acbramley commented
Status: Needs review » Reviewed & tested by the community

MR 17 is looking good, we can enable automated tests after this is merged :)

sunlix made their first commit to this issue’s fork.

sunlix closed merge request !15

  • sunlix committed 6a952928 on 2.x authored by mcaddz 
    Issue #3322662 by JoshaHubbers, sunlix: Update script and listen button...
sunlix’s picture

sunlix
Primary language English
Location Wesel
commented
Status: Reviewed & tested by the community » Fixed

Thanks,

renamed the config key for more clarification.
Added update_hook to initialize the new config key. So no warnings during reading non existend config key from config object should happen. (mostly before first saving the settings form after upgrade)

acbramley’s picture

acbramley commented

Thanks @sunlix! Any chance we could get an RC cut for the 2.x branch?

ressa’s picture

ressa
he/him
commented
Parent issue: » #3349544: Plan for Open ReadSpeaker 8.x-1.10 release

Adding parent issue, it looks like a new release is coming soon :)

  • sunlix committed 6a952928 on 8.x-1.x authored by mcaddz 
    Issue #3322662 by JoshaHubbers, sunlix: Update script and listen button...

Status: Fixed » Closed (fixed)

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