Page MenuHomePhabricator
Create Task
Maniphest T280425

[M] Handle users who arrive on the page with the obsolete "bitmap" parameter
Closed, ResolvedPublic
Actions

Description

Earlier in the development of MediaSearch, the tab for image search results was named bitmap instead of image. It's also the default tab, so many old links (and indexed urls) may send users to the page with this older and no-longer-valid parameter.

Currently this triggers an error which shows up in the logs, and presents the user with the page in an invalid state (no tab selected, no error message or results visible, etc).

Acceptance Criteria
[] Users should see an error message if they land on the page with an invalid type parameter

  • Users who show up on the page with an invalid or missing type parameter should be silently redirected to the Image tab, and their URL should be altered to match.

Note: We discussed this and decided it's no longer necessary: For the special case of type=bitmap, let's automatically redirect the request to be type=image with all other parameters left unchanged.

Related: T277724

Details

SubjectRepoBranchLines +/-
Error handling improvements in JS and PHPmediawiki/extensions/MediaSearchmaster+62 -40
Customize query in gerrit

Related Objects

Event Timeline

egardner created this task.Apr 17 2021, 12:19 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 17 2021, 12:19 AM
CBogen renamed this task from Handle users who arrive on the page with the obsolete "bitmap" parameter to [M] Handle users who arrive on the page with the obsolete "bitmap" parameter.Apr 21 2021, 4:52 PM
CBogen updated the task description. (Show Details)
CBogen moved this task from Ready for Estimation to Ready for Development on the Structured-Data-Backlog (Current Work) board.Apr 23 2021, 2:14 PM
egardner claimed this task.Apr 23 2021, 9:16 PM
gerritbot added a comment.Apr 23 2021, 10:50 PM

Change 682201 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/extensions/MediaSearch@master] Handle missing or invalid tab when JS UI initializes

https://gerrit.wikimedia.org/r/682201

gerritbot added a project: Patch-For-Review.Apr 23 2021, 10:50 PM
egardner moved this task from Ready for Development to Code Review on the Structured-Data-Backlog (Current Work) board.Apr 23 2021, 10:51 PM
egardner added a comment.Apr 23 2021, 11:03 PM

I should note that the open patch here does not exactly align with the acceptance criteria. What happens instead is that users who arrive on the page with an invalid type parameter (?type=bitmap, ?type=foo, etc) will instantly and silently get redirected to (?type=image), the default search type. All other params remain and the UI then loads normally. The URL gets updated to include image as well, so users copying-and-pasting the resulting page will have valid links going forward.

If this behavior is not adequate, we may need to end up re-working how we do error handling here. Right now we are not doing server-side error handling at all.

CBogen subscribed.Apr 26 2021, 1:31 PM
In T280425#7030642, @egardner wrote:

I should note that the open patch here does not exactly align with the acceptance criteria. What happens instead is that users who arrive on the page with an invalid type parameter (?type=bitmap, ?type=foo, etc) will instantly and silently get redirected to (?type=image), the default search type. All other params remain and the UI then loads normally. The URL gets updated to include image as well, so users copying-and-pasting the resulting page will have valid links going forward.

If this behavior is not adequate, we may need to end up re-working how we do error handling here. Right now we are not doing server-side error handling at all.

This sounds good to me!

egardner mentioned this in T281173: Restore server-side error handling to MediaSearch UI.Apr 26 2021, 3:44 PM
egardner added a comment.Apr 26 2021, 11:45 PM

I've opened a separate task (T281173) to address the other gaps in our error-handling behavior. This fix can be applied in the meantime.

egardner updated the task description. (Show Details)Apr 26 2021, 11:48 PM
gerritbot added a comment.May 4 2021, 1:05 PM

Change 682201 merged by jenkins-bot:

[mediawiki/extensions/MediaSearch@master] Error handling improvements in JS and PHP

https://gerrit.wikimedia.org/r/682201

Maintenance_bot removed a project: Patch-For-Review.May 4 2021, 1:10 PM
egardner mentioned this in rEMEDb0955dcbee95: Error handling improvements in JS and PHP.May 4 2021, 1:26 PM
AnneT moved this task from Code Review to Needs QA on the Structured-Data-Backlog (Current Work) board.May 4 2021, 1:56 PM
Etonkovidova moved this task from Needs QA to Verify on Production on the Structured-Data-Backlog (Current Work) board.May 5 2021, 1:26 AM
Etonkovidova subscribed.EditedMay 5 2021, 10:31 PM

Test cases checked in betalabs shows that the js error is present for the cases with invalid values for type and with search parameters. When type is missing entirely - it behaves as expected. I'll re-check in production after all related patches for handling errors will be merged.

DescriptionConsole errors?Screenshot
✅ no search parameters; type is missing https://commons.wikimedia.beta.wmflabs.org/w/index.php?search=&title=Special:MediaSearch&go=Gono errorshttps://commons.wikimedia.beta.wmflabs.org/w/index.php?search=&title=Special:MediaSearch&go=Go&type=image
Screen Shot 2021-05-05 at 3.08.10 PM.png (1×1 px, 140 KB)
✅ no search parameters; type=bitmap https://commons.wikimedia.beta.wmflabs.org/w/index.php?search=&title=Special:MediaSearch&go=Go&type=bitmapno errorsImage tab is not active; no user message https://commons.wikimedia.beta.wmflabs.org/w/index.php?search=&title=Special:MediaSearch&go=Go&type=bitmap
Screen Shot 2021-05-05 at 2.57.14 PM.png (882×1 px, 118 KB)
✅ search parameters present; type is missing https://commons.wikimedia.beta.wmflabs.org/w/index.php?search=roses&title=Special:MediaSearch&go=Gono errorshttps://commons.wikimedia.beta.wmflabs.org/w/index.php?search=roses&title=Special:MediaSearch&go=Go&type=image
Screen Shot 2021-05-05 at 3.15.42 PM.png (1×2 px, 1 MB)
✅ search parameters present; type=bitmap https://commons.wikimedia.beta.wmflabs.org/w/index.php?search=roses&title=Special:MediaSearch&go=Go&type=bitmapTypeError: Cannot read property 'length' of undefinedhttps://commons.wikimedia.beta.wmflabs.org/w/index.php?search=roses&title=Special:MediaSearch&go=Go&type=bitmap
Screen Shot 2021-05-05 at 2.57.14 PM.png (882×1 px, 118 KB)

For type=foo the testing results are the same as for type=bitmap.

egardner added a comment.EditedMay 5 2021, 10:53 PM

@Etonkovidova this patch (like most patches going forward) was applied to the stand-alone MediaSearch extension version of this code – as opposed to the code that lives inside of WikibaseMediaInfo.

Right now the Special:MediaSearch page on Commons (and presumably on Beta) is still powered by the old codebase (which did not get this patch). Sometime in the next week or two, the new extension will go live and the mediasearch code that lives inside of WikibaseMediaInfo will be deleted.

Locally, I have the new extension activated and am able to test things locally at Special:NewMediaSearch. But I don't think the extension is enabled on Beta or Prod even if the code is deployed there.

Until the switchover happens this change (and most other patches currently in flight) may be difficult to test on Beta/Production.

Etonkovidova closed this task as Resolved.Jun 10 2021, 12:23 AM

Re-checked the test cases on commons wmf.9 - all seem to be good now. I've updated my comment above (https://phabricator.wikimedia.org/T280425#7064189) to indicate that all tests are successful now.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL