Thank you so much for finding out simon!! so then nuxt is probably not to blame here
On Tuesday, June 6, 2023 at 8:39:51 PM UTC+5:30 Simon Lecutiez wrote: > 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/121f5466-4db0-4989-bc2c-7d26ff93f478n%40chromium.org.
