Skip to content

Song order cannot be set from the admin; playlist drag-sort may rewrite IDs #1

Description

@Jimmi08

Two ordering problems found while documenting the plugin. Neither is urgent, but both
should be recorded before they are forgotten.

1. Song order is not settable, yet the front end depends on it

jplayer.class.php:339 fetches a playlist's songs with:

$songs = $sql->retrieve("SELECT * FROM $wolf_jplayer_table WHERE playlist_id = '$playlist_id' ORDER BY position", TRUE);

But position is not present in the songs UI $fields array at all (admin_config.php)
— so it can never be set from the admin. Every song keeps the database default of 0, and
MySQL returns tied rows in arbitrary order. Playback order is therefore not controllable.

The songs controller also has no sortField, so there is no drag-to-reorder either.
position is listed in $fieldpref, referring to a field that does not exist in
$fields.

An unused assets/images/admin/move.png icon sits in the plugin, referenced by nothing —
suggesting the WordPress original had drag ordering that was never carried across to the
e107 port.

Fix: add position to the songs $fields array, and/or set sortField on the songs
controller to enable drag ordering.

2. Playlist drag-sort is pointed at the primary key

jplayer_playlists_ui declares:

protected $sortField = 'id';
protected $orderStep = 10;

sortField is meant to name a dedicated ordering column. The jm_jplayer_playlists table
does not have one — so this points e107's drag-sort at the primary key, which songs
reference via jm_jplayer.playlist_id.

Not yet tested. If e107 rewrites id values on reorder, it would orphan every song in
the affected playlists.

Fix: either add a proper ordering column to jm_jplayer_playlists and point
sortField at it, or remove sortField / orderStep from the controller.

Also worth noting while in there

jplayer.class.php:339 interpolates $playlist_id directly into the SQL string. All
current callers intval() it first, so it is not exploitable today — but it is one
careless caller away from being a hole. It should go through a bound parameter.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions