Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Different behavior of autocomplete in production #5770

Closed
tacoWanneeJama opened this issue Oct 8, 2024 · 8 comments
Closed

[Bug]: Different behavior of autocomplete in production #5770

tacoWanneeJama opened this issue Oct 8, 2024 · 8 comments
Assignees
Labels
Type: Bug 🐞 Something isn't working
Milestone

Comments

@tacoWanneeJama
Copy link

tacoWanneeJama commented Oct 8, 2024

Blazorise Version

latest

What Blazorise provider are you running on?

None

Link to minimal reproduction or a simple code snippet

When I run my program in development environment I see this correct behavior:
Image
But when I deploy it on a Linux Azure website I get this strange result:
Image

Steps to reproduce

I did just before a forced clear of caches on both sites

What is expected?

In production the same behavior as in development

What is actually happening?

Wrong filter result

What browsers do you see the problem on?

Chrome (latest)

Any additional comments?

Where should I look for the possible difference between development and production?

@tacoWanneeJama tacoWanneeJama added the Type: Bug 🐞 Something isn't working label Oct 8, 2024
@tacoWanneeJama tacoWanneeJama changed the title [Bug]: DIfferent behahior of autocomplete in production [Bug]: Different behavior of autocomplete in production Oct 9, 2024
@guillaumebaril
Copy link

I have basically the same issue. I have an autocomplete with "AutocompleteFilter.Contains", and it contains around 500 elements. If I do a search for a substring, it's matching correctly and showing the correct results when I'm running in Visual Studio.

However, as soon as I deploy (even if I deploy on IIS locally and test locally), it gives me incorrect matching results only with "fr-CA" culture. If I switch to "en-CA" culture, it's working fine.

I'm on 1.6.2, but I tried on 1.6.1, 1.7.0 and I have the same results.

Seeing your screenshot, my guess is that you're using deutch culture with "AutocompleteFilter.Contains".

Did you manage to find something relating to this issue? I've been scratching my head over this one.

@tacoWanneeJama
Copy link
Author

tacoWanneeJama commented Dec 5, 2024

I managed to get it working ok by using: _streetsFiltered = _streets.Where(x => x.Contains(autocompleteReadDataEventArgs.SearchValue, StringComparison.OrdinalIgnoreCase));
The wrong behavior was when I used compare InvariantCulture. Because we have no dates or so written in the string we can with OrdinalIgnoreCase.

@stsrki stsrki added this to Support Dec 5, 2024
@github-project-automation github-project-automation bot moved this to 🔙 Backlog in Support Dec 5, 2024
@stsrki stsrki added this to the 1.7 support milestone Dec 5, 2024
@guillaumebaril
Copy link

Thanks, it worked. Instead of using Contains filter, I've switched to "Custom filter" and wrote my filter basically like this:

private bool FilterValue(MyModel model, string text)
{
   return model.Name.Contains(text, StringComparaison.OrdinalIgnoreCase);
}

@tesar-tech
Copy link
Collaborator

Hey, I am about to solve the issue in Blazorise.

The "problem" is located here:

where text.IndexOf( Search, 0, StringComparison.CurrentCultureIgnoreCase ) >= 0

It takes the current culture, which is why it yields different results on different machines (depending on their culture settings). This also explains why a custom filter with OrdinalIgnoreCase works.

(I still don't understand why the search term "re" would yield "Waterjuffer." Maybe it's because "er" is inside, but still...)

A few lines below (for StartsWith), it does use OrdinalIgnoreCase:

where text.StartsWith( Search, StringComparison.OrdinalIgnoreCase )

Is there any reason for such a difference, @stsrki?

My Proposal:

  1. Use OrdinalIgnoreCase for both methods, ensuring consistent results across cultures.
  2. Mention in the documentation:
    • "When you need something else, just use CustomFilter."
  3. Mark this as a breaking change for version 2.0.

What do you think?

@stsrki
Copy link
Collaborator

stsrki commented Dec 12, 2024

Is there any reason for such a difference, @stsrki?

I think, @David-Moreira would know better. I believe the difference was by accident.

IMO, the StringComparison.CurrentCultureIgnoreCase should be more correct as it can be controlled by the user system settings and should work better for Unicode strings. However, to make it even better, I would introduce a new API on Autocomplete to control the culture.

[Parameter] public StringComparison StringComparisonCulture { get; set; } = StringComparison.CurrentCultureIgnoreCase;

@David-Moreira
Copy link
Contributor

David-Moreira commented Dec 12, 2024

Personally I typically always use in String Comparison the InvariantCulture or OrdinalIgnoreCase (mostly the ordinal one, which is the most performant) based variants, as I have found that the culture based comparisons are more niche and haven't really found good usage for these, plus they cost a little bit more compute power. Maybe I just haven't worked in an app/project that had use for it.
Specially when it's internal based comparisons where you really just want to check whether the "characters" are the same and you don't want the culture nuances, which more often than not bring "unexpected" results, probably like what you're seeing here.

I must admit I don't fully understand all the intricacies of it, but ordinal based has served me fine in my experience.
Sometimes I refer to these examples for a refresher :
https://learn.microsoft.com/en-us/dotnet/api/system.string.equals?view=net-9.0#system-string-equals(system-string-system-string-system-stringcomparison)

For example note these:

// Current Culture: th-TH
// a = a- (CurrentCulture): True
// a = a- (CurrentCultureIgnoreCase): True
// a = a- (InvariantCulture): False
// a = a- (InvariantCultureIgnoreCase): False
// a = a- (Ordinal): False
// a = a- (OrdinalIgnoreCase): False

// Current Culture: tr-TR
// i = İ (CurrentCulture): False
// i = İ (CurrentCultureIgnoreCase): True
// i = İ (InvariantCulture): False
// i = İ (InvariantCultureIgnoreCase): False
// i = İ (Ordinal): False
// i = İ (OrdinalIgnoreCase): False

For me the lines in bold, I'd just tipically never found a use to have these kind of "culture equality".

So in my opinion, I'd make all string based comparisons OrdinalIgnoreCase or Ordinal (depending whether case is important of course!!) , and give a way to override it. Probably the easiest way is to just provide a global Blazorise option, instead of per component. As I'd imagine if someone really needs to apply a culture based comparison he'd want it to be globally done.

@David-Moreira
Copy link
Contributor

David-Moreira commented Dec 12, 2024

PS:
In this specific case, for the lines that are shown in tesar's comment, it's straight up a bug, since we are using two different comparisons, so it might yield inconsistent results in the component itself.
I'd decide on one (again I agree with @tesar-tech on the OrdinalIgnoreCase) and patch it right away.

We can decide the definitive approach for string comparison for 2.0 (on a separate task!)

@stsrki
Copy link
Collaborator

stsrki commented Dec 12, 2024

David has some valid comments. Yeah, for most use cases, OrdinalIgnoreCase should be the default.

And we can introduce more customizations in 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐞 Something isn't working
Projects
Status: ✔ Done
Development

No branches or pull requests

5 participants