Skip to content

fix: Check embed for "new search" and dedicated landing pages#736

Open
darcywong00 wants to merge 1 commit into
stagingfrom
fix/embed-new-search-dedicated-landings
Open

fix: Check embed for "new search" and dedicated landing pages#736
darcywong00 wants to merge 1 commit into
stagingfrom
fix/embed-new-search-dedicated-landings

Conversation

@darcywong00
Copy link
Copy Markdown
Contributor

@darcywong00 darcywong00 commented May 13, 2026

Follows the page_root pattern in #734 for the "New Search" link.
For Keyman 14.0-18.0 apps using the embedded view, we don't include the locale for "Near Search"

Also added a section in .htaccess so dedicated landing pages can check the embed session cookie.
Fortunately, all the landing pages are index.php.

Test-bot: skip

@darcywong00 darcywong00 added this to the A19S29 milestone May 13, 2026
@darcywong00 darcywong00 requested a review from mcdurdin May 13, 2026 01:59
@github-actions github-actions Bot added the fix label May 13, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Keyman May 13, 2026
@keymanapp-test-bot
Copy link
Copy Markdown

User Test Results

Test specification and instructions

User tests are not required

@darcywong00 darcywong00 force-pushed the fix/embed-new-search-dedicated-landings branch from dbfe69c to 801511c Compare May 13, 2026 02:03
@darcywong00 darcywong00 changed the base branch from fix/user-agent-link-checker to staging May 13, 2026 02:04
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I am not sure how the /h/ pages slipped through because I did test those...

@darcywong00
Copy link
Copy Markdown
Contributor Author

LGTM, I am not sure how the /h/ pages slipped through because I did test those...

hmm, current staging site (before I merge this)

Starting at
https://keyman-staging.com/go/android/14.0/download-keyboards?q=ipa

redirects and settles on https://keyman-staging.com/keyboards/?q=ipa in an embedded search with the cookie embed_keyboards_no_locale_redirect set.

Clicking the top link (landing page to /ipa) goes to

https://keyman-staging.com/en/keyboards/h/ipa/?embed=android&version=10.0.0.0
still embedded view and has 2 cookies

Name Path
embed_keyboards_no_locale_redirect /
embed_keyboards_no_locale_redirect /en/keyboards/h/ipa

@mcdurdin
Copy link
Copy Markdown
Member

Ah I think we need to set the path of the cookie!

@mcdurdin
Copy link
Copy Markdown
Member

    setcookie('embed_keyboards_no_locale_redirect','1',0, '/');

@mcdurdin
Copy link
Copy Markdown
Member

It is important to add the path for the cookie as shown above, but we still need your patch as well. I'll add the cookie as a separate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

3 participants