Page MenuHomePhabricator

Filew/fileh search is broken (always truncated to 1 digit)
Closed, ResolvedPublic8 Story Points

Description

Screenshots


After re-opening the form

Bug description

  • Open the "advanced parameters".
  • Select a file type, e.g. "jpg".
  • Enter either a with, height, or both. Make sure you enter numbers with more than one digit, e.g. "123".
  • Submit.
  • Notice the URL contains search=…+filew:>123&…. This is correct and the reason why the search result is the expected one.
  • But the URL also contains advancedSearch-current={"options":{"filew":[">":"1"]}}. The number here is truncated and only shows the first digit.
  • This is reflected in the form.

Adding one more observation:

  • the information is lost in the form so if we would conduct a new search we'd get different results.

I tried to dig into this and found that all change events except the first one are lost on all 4 file width/height input elements.

Note

  • The two affected fields (ImageDimensionInput) are homebrewn fields that consist of 2 other OOUI elements
  • There is code in ImageDimensionInput that tries to connect the change events of the two operatorInput and valueInput to properly propagate to the store.
  • The change event seems to only listen to the first input, not to any changes after that. However, the correct value appears in the url.
  • There are no unit tests for ImageDimensionInput so far, and they should definitely be added

AcceptanceCriteria
When a number like "1234" is entered into the px field, and then we search,

  • the search results should be correct
  • the search form should still have the same number in the px field
  • there should not be any new params in the search bar
  • the url should not have any unexpected parameters
  • this should work for all px fields in all combinations

Event Timeline

Restricted Application added a project: TCB-Team. · View Herald TranscriptMar 26 2018, 12:59 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Related issues that lead to this report:

  • I can not update my local MediaWiki any more. I get a sequence of "fatal: protocol error: bad pack header".
  • I don't have access to https://tools.wmflabs.org/wmde-uca-test. Why is it so heavily protected in the first place?

I can not update my local MediaWiki any more. I get a sequence of "fatal: protocol error: bad pack header".

This apparently happens every time we delete old wmfXX branches in Gerrit, and is resolved by running git remote prune origin/git remote prune gerrit to remove your local references to those branches. I ran into this myself a few days ago and learned the solution from Chad.

I was able to update my system, thank you very much @matmarex! Unfortunately the OOUI issue remains unaffected, even with the most recent OOUI 0.26.1.

New issue: https://tools.wmflabs.org/wmde-uca-test/ now gives a 404.

Lea_WMDE triaged this task as Normal priority.Mar 28 2018, 1:50 PM
Lea_WMDE updated the task description. (Show Details)
Lea_WMDE set the point value for this task to 8.
Lea_WMDE moved this task from Backlog to Tickets ready for pickup on the Advanced-Search board.
Charlie_WMDE updated the task description. (Show Details)Apr 3 2018, 11:39 AM
gabriel-wmde moved this task from Todo to Doing on the WMDE-FUN-Sprint-2018-04-04 board.

Change 424360 had a related patch set uploaded (by Gabriel Birke; owner: Gabriel Birke):
[mediawiki/extensions/AdvancedSearch@master] Make SearchModel immutable

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

matmarex removed a project: OOUI.Apr 5 2018, 5:52 PM
matmarex removed a subscriber: matmarex.

Change 424612 had a related patch set uploaded (by Gabriel Birke; owner: Gabriel Birke):
[mediawiki/extensions/AdvancedSearch@master] Split AdvancedOptionsBuilder

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

Change 425081 had a related patch set uploaded (by Gabriel Birke; owner: Gabriel Birke):
[mediawiki/extensions/AdvancedSearch@master] Add defaults to search fields

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

gabriel-wmde removed gabriel-wmde as the assignee of this task.Apr 10 2018, 3:26 PM
gabriel-wmde moved this task from Doing to Review on the WMDE-FUN-Sprint-2018-04-04 board.

Change 424360 merged by jenkins-bot:
[mediawiki/extensions/AdvancedSearch@master] Make SearchModel immutable

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

Change 424612 merged by jenkins-bot:
[mediawiki/extensions/AdvancedSearch@master] Split AdvancedOptionsBuilder

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

Change 425081 merged by jenkins-bot:
[mediawiki/extensions/AdvancedSearch@master] Add defaults to search fields

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

The URL is still weird, such as: https://tools.wmflabs.org/wmde-uca-test/core/index.php?search=filetype%3Abitmap+filew%3A%3C4321+fileh%3A%3C4321&title=Spezial:Suche&profile=advanced&fulltext=1&advancedSearch-current=%7B%22options%22%3A%7B%22 filetype%22%3A%22bitmap%22%2C%22filew%22%3A%5B%22%3C%22%2C%224321%22%5D%2C%22fileh%22%3A%5B%22%3C%22%2C%224321%22%5D%7D%2C%22namespaces%22%3A%5B%224%22%2C%226%22%2C%227%22%2C%2212%22%5D%7D&ns4=1&ns6=1&ns7=1&ns12=1&searchToken=4dvggsg5rvw11tcxq6xzkelmz

In decoded form, this URL looks like …?search=filetype:bitmap+filew:<4321+fileh:<4321&title=Spezial:Suche&profile=advanced&fulltext=1&advancedSearch-current={"options":{"filetype":"bitmap","filew":["<","4321"],"fileh":["<","4321"]},"namespaces":["4","6","7","12"]}&ns4=1&ns6=1&ns7=1&ns12=1&searchToken=….

What is particularly "weird" about this?

In my opinion there is not much room for optimization in this URL. All the parameters are needed for one reason or the other. A few possibly optimizations I can see:

  • Get rid of the namespace array in the options, and exclusively use the &ns…=1 parameters for this.
  • Serialize the filew and fileh options as strings. Instead of ["<","4321"] (which becomes %5B%22%3C%22%2C%22…%22%5D when encoded) it will be "<4321" then (encoded %22%3C…%22).

Unrelated: How do I get access to this test wiki?

thiemowmde moved this task from Incoming to Advanced Search on the TCB-Team board.Jun 12 2018, 4:07 PM
Lea_WMDE closed this task as Resolved.Oct 11 2018, 8:52 AM