When adding URLs, new HTML rows should have unique HTML IDs. (#3431)

* When adding URLs, new HTML rows should have unique HTML IDs.
* Fix newly added assertions in Format_General::test_string2htmlid()

Co-authored-by: ྅༻ Ǭɀħ ༄༆ཉ <ozh@ozh.org>

6 files changed, 23 insertions, 12 deletions

diff --git a/admin/admin-ajax.php b/admin/admin-ajax.php
index 103cf1c8..4045f856 100644
--- a/admin/admin-ajax.php
+++ b/admin/admin-ajax.php
@@ -18,7 +18,7 @@ switch( $action ) {
 
 	case 'add':
 		yourls_verify_nonce( 'add_url', $_REQUEST['nonce'], false, 'omg error' );
-		$return = yourls_add_new_link( $_REQUEST['url'], $_REQUEST['keyword'] );
+		$return = yourls_add_new_link( $_REQUEST['url'], $_REQUEST['keyword'], '', $_REQUEST['rowid'] );
 		echo json_encode($return);
 		break;
 
diff --git a/includes/functions-formatting.php b/includes/functions-formatting.php
index 83fc396d..49a9a75e 100644
--- a/includes/functions-formatting.php
+++ b/includes/functions-formatting.php
@@ -52,12 +52,16 @@ function yourls_string2int($string, $chars = null) {
  * Return a unique string to be used as a valid HTML id
  *
  * @since   1.8.3
- * @param  string $prefix   Optional prefix
- * @return string           The unique string
- */
-function yourls_unique_element_id($prefix = 'yid') {
-    static $id_counter = 0;
-	return yourls_apply_filter( 'unique_element_id', $prefix . (string)++$id_counter );
+ * @param  string $prefix      Optional prefix
+ * @param  int    $initial_val The initial counter value (defaults to one)
+ * @return string              The unique string
+ */
+function yourls_unique_element_id($prefix = 'yid', $initial_val = 1) {
+    static $id_counter = 1;
+	if ($initial_val > 1) {
+		$id_counter = (int) $initial_val;
+	}
+	return yourls_apply_filter( 'unique_element_id', $prefix . (string) $id_counter++ );
 }
 
 /**
diff --git a/includes/functions-html.php b/includes/functions-html.php
index a619ffdc..427e28eb 100644
--- a/includes/functions-html.php
+++ b/includes/functions-html.php
@@ -545,11 +545,12 @@ RETURN;
  * @param string $ip          IP
  * @param string|int $clicks  Number of clicks
  * @param string $timestamp   Timestamp
+ * @param int    $row_id      Numeric value used to form row IDs, defaults to one
  * @return string             HTML of the row
  */
-function yourls_table_add_row( $keyword, $url, $title, $ip, $clicks, $timestamp ) {
+function yourls_table_add_row( $keyword, $url, $title, $ip, $clicks, $timestamp, $row_id = 1 ) {
 	$keyword  = yourls_sanitize_keyword($keyword);
-	$id       = yourls_unique_element_id();
+	$id       = yourls_unique_element_id('yid', $row_id);
 	$shorturl = yourls_link( $keyword );
 
 	$statlink = yourls_statlink( $keyword );
diff --git a/includes/functions-shorturls.php b/includes/functions-shorturls.php
index 93dffb74..3fa74d96 100644
--- a/includes/functions-shorturls.php
+++ b/includes/functions-shorturls.php
@@ -26,9 +26,10 @@
  * @param  string $url      URL to shorten
  * @param  string $keyword  optional "keyword"
  * @param  string $title    option title
+ * @param  int    $row_id   used to form unique IDs in the generated HTML
  * @return array            array with error/success state and short URL information
  */
-function yourls_add_new_link( $url, $keyword = '', $title = '' ) {
+function yourls_add_new_link( $url, $keyword = '', $title = '', $row_id = 1 ) {
     // Allow plugins to short-circuit the whole function
     $pre = yourls_apply_filter( 'shunt_add_new_link', false, $url, $keyword, $title );
     if ( false !== $pre ) {
@@ -140,7 +141,7 @@ function yourls_add_new_link( $url, $keyword = '', $title = '' ) {
             $return['status']   = 'success';
             $return['message']  = /* //translators: eg "http://someurl/ added to DB" */ yourls_s( '%s added to database', yourls_trim_long_string( $url ) );
             $return['title']    = $title;
-            $return['html']     = yourls_table_add_row( $keyword, $url, $title, $ip, 0, time() );
+            $return['html']     = yourls_table_add_row( $keyword, $url, $title, $ip, 0, time(), $row_id );
             $return['shorturl'] = yourls_link($keyword);
             $return['statusCode'] = 200; // 200 OK
         } else {
diff --git a/js/insert.js b/js/insert.js
index 8298f7fe..03e004e7 100644
--- a/js/insert.js
+++ b/js/insert.js
@@ -35,10 +35,11 @@ function add_link() {
 		return;
 	}
 	var keyword = $("#add-keyword").val();
+	var nextid = parseInt($('#main_table tbody tr[id^="id-"]').length) + 1;
 	add_loading("#add-button");
 	$.getJSON(
 		ajaxurl,
-		{action:'add', url: newurl, keyword: keyword, nonce: nonce},
+		{action:'add', url: newurl, keyword: keyword, nonce: nonce, rowid: nextid},
 		function(data){
 			if(data.status == 'success') {
 				$('#main_table tbody').prepend( data.html ).trigger("update");
diff --git a/tests/tests/format/general.php b/tests/tests/format/general.php
index 3dbf834d..74e0ee35 100644
--- a/tests/tests/format/general.php
+++ b/tests/tests/format/general.php
@@ -105,9 +105,13 @@ class Format_General extends PHPUnit\Framework\TestCase {
     public function test_string2htmlid() {
         $id1 = yourls_unique_element_id();
         $id2 = yourls_unique_element_id();
+        $id3 = yourls_unique_element_id('foo', 10);
+        $id4 = yourls_unique_element_id();
         $this->assertIsString($id1);
         $this->assertIsString($id2);
         $this->assertNotSame($id1, $id2);
+        $this->assertEquals('foo10', $id3, 'ID is built using the specified prefix and counter value.');
+        $this->assertStringEndsWith('11', $id4, 'ID counter continues to increment from the last value.');
     }
 
     /**