Hello, > I am still analyzing if this is a library issue or maybe its some specific style of writing nuxt
Both websites were made by the same agency (Akaru <https://akaru.fr/>), so it could even be a homemade library or a single developer’s style ;) Hope it helps, Simon Le lundi 5 juin 2023 à 17:07:47 UTC+2, Debadree Chatterjee a écrit : > Hey! > > I have attached the difference in how they look for ttps://maisonyoko.com/ > <https://maisonyoko.com/> note that the background video doesn't load > maybe since for local testing I am raising exceptions thats why its the > causing issue (I am checking this) as for the nuxt issue I am still > analyzing if this is a library issue or maybe its some specific style of > writing nuxt > > As for the CL i shall then include both the runtime flag and use counter > in the same CL hopefully by tomorrow > > Regards, > Debadree > On Monday, June 5, 2023 at 7:47:44 PM UTC+5:30 Philip Jägenstedt wrote: > >> Hi Debadree, >> >> I also noticed the code was similar for multiple sites, I'm glad you were >> able to track it down to a common framework, Nuxt. I would suggest filing >> an issue at https://github.com/nuxt/nuxt about this problem, maybe they >> can fix it in the next release. Since you've confirmed that some sites >> won't load the video correctly, and fixing it would require upgrading the >> framework, it's important to have a fix out there by the time the change is >> done in Chrome. >> >> Since the WebKit change <https://github.com/WebKit/WebKit/pull/13500> >> was included in STP 171 >> <https://developer.apple.com/documentation/safari-technology-preview-release-notes/stp-release-171>, >> >> I tested https://maisonyoko.com/ and https://crossfitdespentes.fr/ in >> both Safari and STP 171, but I couldn't see any difference. Which video >> should I expect to not load as a result of this change? I was expecting >> something to be broken, and then I'd file a WebKit regression bug about it. >> >> As for whether to bake the changes into a single CL or to split it, I >> don't have a strong opinion. >> >> Best regards, >> Philip >> >> On Sun, Jun 4, 2023 at 9:05 PM Debadree Chatterjee <debad...@gmail.com> >> wrote: >> >>> Hey! >>> >>> So I did a more thorough testing! and it seems there is something common >>> in both these two sites they are using the Nuxt Framework and the breakage >>> in both of them is similar they have videos on the first section of their >>> landing page with animations on top of them, due to the breakage it seems >>> the video doesn't load! If you look at the call sites too the code looks >>> very similar >>> >>> for https://maisonyoko.com/ >>> [image: Screenshot 2023-06-05 at 12.24.34 AM.png] >>> >>> for https://crossfitdespentes.fr/ >>> [image: Screenshot 2023-06-05 at 12.25.09 AM.png] >>> >>> Both of them nuxt >>> [image: Screenshot 2023-06-05 at 12.27.12 AM.png] >>> [image: Screenshot 2023-06-05 at 12.27.50 AM.png] >>> >>> I am not familiar with nuxt I will try to get to know about it, but it >>> seems like there could be some problem with nuxt, If any reader here knows >>> nuxt would love to reach out! >>> >>> Thank You >>> Debadree >>> On Wednesday, May 31, 2023 at 9:38:18 PM UTC+5:30 Debadree Chatterjee >>> wrote: >>> >>>> Hey Philip! >>>> >>>> In my initial testing, I didn't see any observable change in site >>>> behavior, but I shall confirm once again! >>>> >>>> As for the kill switch and use counter should I update my existing CL >>>> to include these or make a new one containing the counter to just measure >>>> the effects? >>>> >>>> >>>> On Wednesday, May 31, 2023 at 9:28:39 PM UTC+5:30 Philip Jägenstedt >>>> wrote: >>>> >>>>> Hi Debadree, >>>>> >>>>> Thank you so much for your hard work on this. >>>>> >>>>> To confirm, these two sites were from the 20 listed earlier in this >>>>> thread, is that right? Now that we've confirmed that these two sites will >>>>> have a different behavior, can you observe any difference on >>>>> https://maisonyoko.com/ or https://crossfitdespentes.fr/ with the >>>>> changes, but without the exception-throwing? >>>>> >>>>> Generally, finding a behavior change in 10% of tested sites is a bit >>>>> concerning, but if it means that only ~10% of the cases hit by the use >>>>> counter >>>>> <https://chromestatus.com/metrics/feature/timeline/popularity/4478> >>>>> are problematic, it could be <0.001% of sites, and we've >>>>> successfully made breaking changes at that level in the past. >>>>> >>>>> Since you now have the code for throwing an exception, would it be >>>>> straightforward to turn that into a use counter that we can land and get >>>>> a >>>>> better measure of this? I think as discussed previously in this thread, >>>>> we >>>>> should considering shipping this with a killswitch and a use counter, so >>>>> we >>>>> can both revert and check the usage if we get reports of breakage. >>>>> >>>>> Best regards, >>>>> Philip >>>>> >>>>> On Tue, May 30, 2023 at 5:16 PM Debadree Chatterjee < >>>>> debad...@gmail.com> wrote: >>>>> >>>>>> Hey Philip! >>>>>> >>>>>> Even I was surprised, turns out I was wrong about the delete function >>>>>> (so sorry), I have observed a breakage now, >>>>>> >>>>>> The has function output example: >>>>>> [image: Screenshot 2023-05-30 at 7.36.36 PM.png] >>>>>> >>>>>> >>>>>> The updated delete function: >>>>>> >>>>>> ```c++ >>>>>> void URLSearchParams::deleteAllWithNameOrTuple(const String& name, >>>>>> const String& value, >>>>>> ExceptionState& exception_state) { >>>>>> std::vector<int> indices_to_remove_with_name_val, >>>>>> indices_to_remove_with_name; >>>>>> for (wtf_size_t i = 0; i < params_.size(); i++) { >>>>>> if (params_[i].first == name && >>>>>> (value.IsNull() || params_[i].second == value)) { >>>>>> indices_to_remove_with_name_val.push_back(i); >>>>>> } >>>>>> } >>>>>> >>>>>> for (wtf_size_t i = 0; i < params_.size(); i++) { >>>>>> if (params_[i].first == name) { >>>>>> indices_to_remove_with_name.push_back(i); >>>>>> } >>>>>> } >>>>>> >>>>>> if (indices_to_remove_with_name_val.size() != >>>>>> indices_to_remove_with_name.size()) { >>>>>> DLOG(ERROR) << "indices_to_remove_with_name_val.size() != >>>>>> indices_to_remove_with_name.size()"; >>>>>> exception_state.ThrowException(1, "Divergent behavior"); >>>>>> return; >>>>>> } >>>>>> >>>>>> // match the values of indices_to_remove_with_name_val, >>>>>> indices_to_remove_with_name >>>>>> for (size_t i = 0; i < indices_to_remove_with_name_val.size(); i++) >>>>>> { >>>>>> if (indices_to_remove_with_name_val[i] != >>>>>> indices_to_remove_with_name[i]) { >>>>>> DLOG(ERROR) << "indices_to_remove_with_name_val[i] != >>>>>> indices_to_remove_with_name[i]"; >>>>>> exception_state.ThrowException(1, "Divergent behavior"); >>>>>> return; >>>>>> } >>>>>> } >>>>>> >>>>>> for (wtf_size_t i = 0; i < params_.size();) { >>>>>> if (params_[i].first == name && >>>>>> (value.IsNull() || params_[i].second == value)) { >>>>>> params_.EraseAt(i); >>>>>> } else { >>>>>> i++; >>>>>> } >>>>>> } >>>>>> >>>>>> RunUpdateSteps(); >>>>>> } >>>>>> ``` >>>>>> The example outputs of the delete function: >>>>>> [image: Screenshot 2023-05-30 at 8.39.03 PM.png] >>>>>> >>>>>> Example Breakages: >>>>>> In https://maisonyoko.com/ >>>>>> [image: Screenshot 2023-05-30 at 8.33.59 PM.png] >>>>>> https://crossfitdespentes.fr/ >>>>>> [image: Screenshot 2023-05-30 at 8.42.01 PM.png] >>>>>> Other than these sites didn't notice a breakage. >>>>>> >>>>>> seems like the breakages seem to be in the delete function only, so >>>>>> sorry once again for the mistake before. Do let me know if all this >>>>>> looks >>>>>> ok. >>>>>> >>>>>> Thank You, >>>>>> Debadree >>>>>> >>>>>> On Tuesday, May 30, 2023 at 5:34:18 PM UTC+5:30 Philip Jägenstedt >>>>>> wrote: >>>>>> >>>>>>> Hi Debadree, >>>>>>> >>>>>>> That's very promising! The code looks right to me, but just to be >>>>>>> sure, did you verify that the exceptions are thrown in a test case >>>>>>> where >>>>>>> the 2nd argument makes a difference? It's a bit suspicious when no >>>>>>> sites at >>>>>>> all threw the exception :) >>>>>>> >>>>>>> Best regards, >>>>>>> Philip >>>>>>> >>>>>>> On Tue, May 30, 2023 at 11:45 AM Debadree Chatterjee < >>>>>>> debad...@gmail.com> wrote: >>>>>>> >>>>>>>> Hey Everyone! >>>>>>>> >>>>>>>> Sorry for the delays I followed Philip's suggestion on testing if >>>>>>>> behavior diverged in these sites, I checked this by throwing >>>>>>>> exceptions if >>>>>>>> the actual return value is different if I used name only or both name >>>>>>>> and >>>>>>>> value, I am including the code for reference: >>>>>>>> >>>>>>>> bool URLSearchParams::has(const String& name, const String& value, >>>>>>>> ExceptionState& exception_state) const { bool found_match_name = >>>>>>>> false, found_match_name_value = false; for (const auto& param : >>>>>>>> params_) { if (param.first == name) { found_match_name = true; >>>>>>>> break; } } for (const auto& param : params_) { if (param.first == >>>>>>>> name && (value.IsNull() || param.second == value)) { >>>>>>>> found_match_name_value >>>>>>>> = true; break; } } if (found_match_name != found_match_name_value) >>>>>>>> { exception_state.ThrowException(1, "Divergent behavior"); return >>>>>>>> false; } return found_match_name_value; } void >>>>>>>> URLSearchParams::deleteAllWithNameOrTuple(const String& name, const >>>>>>>> String& value, ExceptionState& exception_state) { for (wtf_size_t >>>>>>>> i = 0; i < params_.size();) { bool match_by_name = >>>>>>>> params_[i].first == name; bool match_by_value = !value.IsNull() && >>>>>>>> params_[i].second == value; if (match_by_name) { if >>>>>>>> (match_by_value) { params_.EraseAt(i); } else { // It would have >>>>>>>> been deleted if value wasnt there exception_state.ThrowException(1, >>>>>>>> "Divergent behavior"); return; } } else { i++; } } >>>>>>>> RunUpdateSteps(); } >>>>>>>> >>>>>>>> The good news is none of the example sites broke or triggered this >>>>>>>> exception at all, I navigated lightly through all the sites but no >>>>>>>> exception was observed, whereas if I raised an exception whenever >>>>>>>> double >>>>>>>> variables are passed all the sites would give an exception as you have >>>>>>>> seen >>>>>>>> in the previous mails. Do let me know if this seems correct! >>>>>>>> >>>>>>>> Regards, >>>>>>>> Debadree >>>>>>>> >>>>>>>> On Wednesday, May 24, 2023 at 10:35:59 PM UTC+5:30 Debadree >>>>>>>> Chatterjee wrote: >>>>>>>> >>>>>>>>> Understood! >>>>>>>>> >>>>>>>>> I am going with the local testing approach for now, I think what I >>>>>>>>> will do is raise exceptions if a difference in behavior is noted as >>>>>>>>> Philip >>>>>>>>> suggested, and see how many of these example sites raise them. This >>>>>>>>> may >>>>>>>>> take a little bit of time I think but trying my best! >>>>>>>>> >>>>>>>>> Thank You! >>>>>>>>> >>>>>>>>> On Wednesday, May 24, 2023 at 9:13:04 PM UTC+5:30 Philip >>>>>>>>> Jägenstedt wrote: >>>>>>>>> >>>>>>>>>> If refining the use counter is easy, that would be good to do, >>>>>>>>>> even if we don't block shipping on getting stable data for the use >>>>>>>>>> counter. >>>>>>>>>> >>>>>>>>>> But I think that careful local testing is the best way to get a >>>>>>>>>> sense for the risk on this. If you're confident you've hit the code >>>>>>>>>> path on >>>>>>>>>> the sites in question, and nothing at all changes for the user, then >>>>>>>>>> I >>>>>>>>>> think we should try to ship this. >>>>>>>>>> >>>>>>>>>> On Mon, May 22, 2023 at 6:59 PM Debadree Chatterjee < >>>>>>>>>> debad...@gmail.com> wrote: >>>>>>>>>> >>>>>>>>>>> For basic testing of the sites, I saw no breaking behavior, I >>>>>>>>>>> did a few actions on sites like adding things to the cart, trying >>>>>>>>>>> to go the >>>>>>>>>>> login flow clicking on navigation, etc. Although I think would need >>>>>>>>>>> to go a >>>>>>>>>>> little deep on that, Should I submit a new CL for this counter >>>>>>>>>>> thing? or do >>>>>>>>>>> deeper local testing? >>>>>>>>>>> >>>>>>>>>>> On Monday, May 22, 2023 at 10:09:26 PM UTC+5:30 Philip >>>>>>>>>>> Jägenstedt wrote: >>>>>>>>>>> >>>>>>>>>>>> Well, this is a tricky case with no obvious answer. You've >>>>>>>>>>>> found one case of array.some(...), which most likely will change >>>>>>>>>>>> the >>>>>>>>>>>> behavior of the code. For the other cases where a second argument >>>>>>>>>>>> is passed >>>>>>>>>>>> is explicitly, it depends on the value whether it changes >>>>>>>>>>>> behavior, if it's >>>>>>>>>>>> the same value that was added, then it's fine. >>>>>>>>>>>> >>>>>>>>>>>> One concrete thing you could do is to refine the use counter to >>>>>>>>>>>> only count the cases where the 2nd argument results in has() >>>>>>>>>>>> returning >>>>>>>>>>>> false instead of true, or where delete() doesn't delete anything >>>>>>>>>>>> but would >>>>>>>>>>>> without the 2nd argument. However, I'm not sure that would be >>>>>>>>>>>> informative, >>>>>>>>>>>> if it reduces the use counter by 10x we'd still be unsure about >>>>>>>>>>>> how serious >>>>>>>>>>>> the breakage is to users. >>>>>>>>>>>> >>>>>>>>>>>> In your manual testing of these sites, were you able to confirm >>>>>>>>>>>> the code paths were taken, and unable to spot anything at all >>>>>>>>>>>> broken on the >>>>>>>>>>>> pages? Did you compare to how the sites work without the changes? >>>>>>>>>>>> >>>>>>>>>>>> I would say that given testing of sites that hit the code path, >>>>>>>>>>>> if you can't find anything at all breaking, then we should try to >>>>>>>>>>>> ship the >>>>>>>>>>>> change. >>>>>>>>>>>> >>>>>>>>>>>> On Fri, May 19, 2023 at 3:40 PM Debadree Chatterjee < >>>>>>>>>>>> debad...@gmail.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> I tried navigating and clicking around the sites, but they >>>>>>>>>>>>> didn't seem to be breaking atleast even though this exception is >>>>>>>>>>>>> being >>>>>>>>>>>>> raised. Are there any more investigations I can do? >>>>>>>>>>>>> >>>>>>>>>>>>> On Friday, May 19, 2023 at 3:59:21 AM UTC+5:30 >>>>>>>>>>>>> abot...@igalia.com wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> As for having a premonition that this would be added, there >>>>>>>>>>>>>> is at least one post in the original Github issue saying that >>>>>>>>>>>>>> the poster >>>>>>>>>>>>>> already expected the two-argument overload to be supported ( >>>>>>>>>>>>>> https://github.com/whatwg/url/issues/335#issuecomment-919700370 >>>>>>>>>>>>>> ). >>>>>>>>>>>>>> >>>>>>>>>>>>>> Andreu >>>>>>>>>>>>>> On 5/18/23 23:42, PhistucK wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>> Most of them are just weird, really. I can only imagine they >>>>>>>>>>>>>> started with a .set with an empty string as a second >>>>>>>>>>>>>> parameter and ended up changing to .delete without deleting >>>>>>>>>>>>>> the second parameter. >>>>>>>>>>>>>> (Or they had a premonition and knew there will be a second >>>>>>>>>>>>>> parameter with the specific purpose you want to ship hehe) >>>>>>>>>>>>>> >>>>>>>>>>>>>> I imagine those were outliers, I would not worry much about >>>>>>>>>>>>>> it (also the bound callback is a bit too convoluted to be widely >>>>>>>>>>>>>> used), but >>>>>>>>>>>>>> that is just me. :) >>>>>>>>>>>>>> >>>>>>>>>>>>>> -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/256a149f-8546-43a4-ae42-5370d18ff95bn%40chromium.org.
