diff --git a/docs/todo/CLAUDE.md b/docs/todo/CLAUDE.md index c0d711092df..bc9a0f32e83 100644 --- a/docs/todo/CLAUDE.md +++ b/docs/todo/CLAUDE.md @@ -26,6 +26,8 @@ without re-doing the investigation. Convert relative dates to absolute. Update `Status` when the work lands. +Once `Status` is final (applied or rejected), move the file to `archive/`. + ## Voice Write for any reader, not for the person who requested the review. diff --git a/docs/todo/pocketarc-3.4.4-incorporation.md b/docs/todo/archive/pocketarc-3.4.4-incorporation.md similarity index 80% rename from docs/todo/pocketarc-3.4.4-incorporation.md rename to docs/todo/archive/pocketarc-3.4.4-incorporation.md index cef1ec68593..4065aec632c 100644 --- a/docs/todo/pocketarc-3.4.4-incorporation.md +++ b/docs/todo/archive/pocketarc-3.4.4-incorporation.md @@ -3,7 +3,10 @@ **Release:** https://github.com/pocketarc/codeigniter/releases/tag/3.4.4 (commit `fb57d35`, 18 Jul) **Target:** this repository (CodeIgniter 3.1-stable derivative) **Reviewed:** 2026-08-21 -**Status:** Not actioned. One functional gap (#48), one test gap (#50). +**Status:** Applied 2026-10-06 in `7d676952c` — `is_array()` guard +(section 2) and `test_edge()` (section 5). Key change deferred; see section 6. +**Verdict:** Two items already present; the deprecation guard and the Edge test were +the only gaps, and both are now in place. --- @@ -76,6 +79,16 @@ elseif ($file[0] !== '.') This guards the assignment while preserving the existing basename keys. +**Decided: applied as above** in +[file_helper.php:251-258](system/helpers/file_helper.php#L251-L258). Entries for +which `get_file_info()` fails are now omitted from the result rather than stored +as `FALSE`. + +No unit test was added. `get_dir_file_info()` calls `realpath()` on its first +call, which does not resolve `vfs://` paths, and a dangling symlink on the real +filesystem needs elevated privileges on Windows. The existing `file_helper` tests +pass. + ### PR #48 is not directly applicable — two obstacles 1. **It references `$_root_dir`**, a variable that does not exist in this @@ -157,17 +170,32 @@ Porting it is worthwhile: it pins ordering behaviour that is easy to break by editing `user_agents.php`. The expected legacy label needs adjusting to `Edge Legacy (Spartan)` to match the local config. +**Decided: ported** as `test_edge()` in +[Useragent_test.php](tests/codeigniter/libraries/Useragent_test.php), covering all +three agent strings in the table above (browser label and version). + +### `is_browser()` takes the key, not the label + +Found while writing the test. +[User_agent.php:431](system/libraries/User_agent.php#L431) compares +`$this->browsers[$key]` against the detected label, so the argument is the +**config key**. For Chromium Edge, `is_browser('Edg')` is `TRUE` and +`is_browser('Edge')` is `FALSE`, because the `Edge` key maps to the legacy label. +Callers that test for modern Edge with `is_browser('Edge')` get the wrong answer. +This behaviour is inherited from upstream and is not changed here; the test +asserts `is_browser('Edg')` to document it. + --- ## 6. Recommended order of work 1. **Add the `is_array()` guard in `get_dir_file_info()`** — the only functional gap. Small, no BC break, and it clears a PHP 8.1 deprecation that becomes - fatal under PHP 9. + fatal under PHP 9. **Done.** 2. **Port `test_edge()`** — inexpensive regression protection for behaviour that - is already correct. + is already correct. **Done.** 3. **Defer** the basename-to-relative-path key change. It fixes a real collision - defect but breaks the array contract; decide separately. + defect but breaks the array contract; decide separately. **Deferred.** Nothing in 3.4.4 requires the Edge configuration or `write_log` changes; both are already in place. diff --git a/system/helpers/file_helper.php b/system/helpers/file_helper.php index a2adaf296ed..45f89d8e09f 100644 --- a/system/helpers/file_helper.php +++ b/system/helpers/file_helper.php @@ -248,8 +248,14 @@ function get_dir_file_info($source_dir, $top_level_only = TRUE, $_recursion = FA } elseif ($file[0] !== '.') { - $_filedata[$file] = get_file_info($source_dir.$file); - $_filedata[$file]['relative_path'] = $relative_path; + $filedata = get_file_info($source_dir.$file); + + // get_file_info() returns FALSE for e.g. broken symlinks + if (is_array($filedata)) + { + $filedata['relative_path'] = $relative_path; + $_filedata[$file] = $filedata; + } } } diff --git a/tests/codeigniter/libraries/Useragent_test.php b/tests/codeigniter/libraries/Useragent_test.php index 087544a1a50..07cf26e60c9 100644 --- a/tests/codeigniter/libraries/Useragent_test.php +++ b/tests/codeigniter/libraries/Useragent_test.php @@ -122,4 +122,27 @@ public function test_parse() $this->assertTrue($this->agent->is_mobile('android')); } + // -------------------------------------------------------------------- + + public function test_edge() + { + // Chromium-based Edge (desktop) - must not be reported as Chrome + $this->agent->parse('Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36 Edg/91.0.864.59'); + $this->assertEquals('Edge', $this->agent->browser()); + $this->assertEquals('91.0.864.59', $this->agent->version()); + // is_browser() takes the config key, not the label + $this->assertTrue($this->agent->is_browser('Edg')); + $this->assertFalse($this->agent->is_browser('Chrome')); + + // Legacy EdgeHTML (Spartan) + $this->agent->parse('Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/70.0.3538.102 Safari/537.36 Edge/18.18363'); + $this->assertEquals('Edge Legacy (Spartan)', $this->agent->browser()); + $this->assertEquals('18.18363', $this->agent->version()); + + // Chromium-based Edge (Android) + $this->agent->parse('Mozilla/5.0 (Linux; Android 10; K) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 EdgA/120.0.0.0'); + $this->assertEquals('Edge', $this->agent->browser()); + $this->assertEquals('120.0.0.0', $this->agent->version()); + } + } \ No newline at end of file