Upd. Contact encoder. Refactoring for the code duplication removing.

[Glomberg]

https://app.doboard.com/1/task/41418

[Copilot]

Pull request overview

This PR refactors the ContactsEncoder implementation to reduce duplicated logic, primarily by removing PHP-version-specific branches and simplifying how match context is handled during global email/phone replacements.

Changes:

• Removed the unused plain_email_pattern_without_capturing property and its corresponding test.
• Consolidated modifyGlobalEmails() / modifyGlobalPhoneNumbers() logic by removing PHP 7.4+ “V2” implementations and version branches.
• Updated ContactsEncoderHelper APIs to accept plain strings instead of PREG_OFFSET_CAPTURE match arrays.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
ContactsEncoder.php	Removes version-specific callback branches and V2 link encoders; centralizes email/phone replacement logic.
Helper/ContactsEncoderHelper.php	Changes helper method signatures to accept email strings and uses strpos() to find positions.
tests/ContactsEncoder/TestContactsEncoderPatterns.php	Removes the test for the deleted regex property.

Suppressed comments (1)

ContactsEncoder.php:378

• isInsideScriptTag() is called with $matches[0][0], which is only the first character of the matched phone string. This makes the script-tag containment check unreliable.

                    // check if in script
                    if ( $this->helper->isInsideScriptTag($matches[0][0], $this->temp_content) ) {
                        return $matches[0];

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (4)

Helper/ContactsEncoderHelper.php:60

• isMailtoAdditionalCopy() now derives the match position via strpos($content, $email), which always returns the first occurrence. If the same email appears multiple times, later matches can be misclassified (e.g., if the first occurrence is in cc=/bcc= but a later one isn’t, the later one will still be treated as an additional copy). This method needs the position of the current regex match (or an equivalent per-match offset) to be reliable.

    public function isMailtoAdditionalCopy($email, $content)
    {
        $position = strpos($content, $email);

        if ($position === false) {
            return false;
        }

ContactsEncoder.php:323

• modifyGlobalEmails() now encodes any match containing mailto: by replacing just the mailto:<email> substring with a string that injects " data-original-string=... title=".... This assumes the match is inside an HTML attribute value and is immediately followed by a closing quote. If mailto: appears in plain text, or if the href contains query params (e.g. mailto:a@b.com?subject=Hi&cc=c@d.com), this replacement can corrupt the surrounding HTML/attribute structure.

            if (
                isset($matches[0]) && $this->helper->isMailtoAdditionalCopy($matches[0], $this->temp_content)
            ) {
                return '';
            }

            if ( isset($matches[0]) &&  $this->helper->isMailto($matches[0]) ) {
                return $this->encodeMailtoLink($matches[0]);
            }

ContactsEncoder.php:355

• modifyGlobalPhoneNumbers() encodes any match containing tel: by replacing just the matched tel:+<digits> substring with a string that injects " data-original-string=... title=".... This assumes the match is inside an href attribute value and is immediately followed by a closing quote; if tel: appears in plain text (or any non-attribute context), the replacement will introduce stray quotes/attributes and can break markup.

                if ( isset($matches[0]) ) {
                    if ( $this->helper->isTelTag($matches[0]) ) {
                        return $this->encodeTelLink($matches[0]);
                    }

Helper/ContactsEncoderHelper.php:50

• The docblock for isMailtoAdditionalCopy() says it checks whether the string contains a mailto: link, but the implementation actually checks whether the email is immediately preceded by cc= or bcc= in the content. Updating the docblock will prevent confusion for future maintainers.

    /**
     * Checking if the string contains mailto: link
     *
     * @param string $email
     * @param string $content