Thanks for cleaning this little wart up. LGTM2 On Wed, Dec 4, 2024 at 10:24 AM Stephen Chenney <schen...@chromium.org> wrote:
> I don't get to give LGTMs but after investigation and offline discussion > there seems to be essentially zero breakage risk, so I see no problems at > all with shipping this. > > On Wed, Dec 4, 2024 at 9:56 AM Daniel Bratell <bratel...@gmail.com> wrote: > >> LGTM1 >> >> It makes sense to just make Dominic's HTML persona happy by keeping >> <area> and <a> in sync. >> >> /Daniel >> On 2024-12-04 14:24, Andrew Paseltiner wrote: >> >> >> >> On Tue, Dec 3, 2024 at 9:31 PM Stephen Chenney <schen...@chromium.org> >> wrote: >> >>> >>> >>> On Tue, Dec 3, 2024 at 8:13 PM Vladimir Levin <vmp...@chromium.org> >>> wrote: >>> >>>> >>>> >>>> On Tue, Dec 3, 2024 at 10:34 AM Stephen Chenney <schen...@chromium.org> >>>> wrote: >>>> >>>>> >>>>> >>>>> On Mon, Dec 2, 2024 at 11:38 AM Andrew Paseltiner < >>>>> apaselti...@chromium.org> wrote: >>>>> >>>>>> On Thu, Nov 21, 2024 at 9:50 PM Domenic Denicola < >>>>>> dome...@chromium.org> wrote: >>>>>> >>>>>>> With my HTML editor hat on, I support keeping parity between <a> and >>>>>>> <area>. Although <area> is used much less, we try to keep them symmetric >>>>>>> whenever possible. >>>>>>> >>>>>> Sounds to me like we should go ahead with this for parity. >>>>>> >>>>>>> >>>>>>> On Fri, Nov 22, 2024 at 5:19 AM Mike Taylor <miketa...@chromium.org> >>>>>>> wrote: >>>>>>> >>>>>>>> On 11/21/24 1:37 PM, Andrew Paseltiner wrote: >>>>>>>> >>>>>>>> On Thu, Nov 21, 2024 at 1:26 PM Mike Taylor <miketa...@chromium.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> On 11/21/24 12:49 PM, Chromestatus wrote: >>>>>>>>> >>>>>>>>> Contact emails apaselti...@chromium.org >>>>>>>>> >>>>>>>>> Explainer >>>>>>>>> https://github.com/WICG/attribution-reporting-api/blob/main/EVENT.md#registering-attribution-sources >>>>>>>>> >>>>>>>>> Specification >>>>>>>>> https://wicg.github.io/attribution-reporting-api/#html-monkeypatches >>>>>>>>> >>>>>>>>> Summary >>>>>>>>> >>>>>>>>> For Attribution Reporting, the attributionsrc attribute was >>>>>>>>> already unintentionally processed on <area> elements due to code >>>>>>>>> shared >>>>>>>>> with <a>, which intentionally supported that attribute. For >>>>>>>>> completeness, >>>>>>>>> we expose the attribute on <area> with identical syntax and semantics >>>>>>>>> to >>>>>>>>> <a> and without changing the previous processing: When an <area> tag >>>>>>>>> with >>>>>>>>> an attributionsrc attribute is navigated, the foreground request may >>>>>>>>> register navigation sources and, if the attribute is non-empty, one >>>>>>>>> or more >>>>>>>>> background requests will likewise be able to register navigation >>>>>>>>> sources. >>>>>>>>> >>>>>>>>> Is this something developers actually want, i.e. are imagemaps a >>>>>>>>> use case advertisers are asking to be supported? If not, why not just >>>>>>>>> fix >>>>>>>>> what seems to be a bug? >>>>>>>>> >>>>>>>> It's true that we haven't specifically heard from developers that >>>>>>>> they want this, but we also don't have any data about whether the >>>>>>>> existing >>>>>>>> behavior is being relied on, and I'm not clear on the prevalence of >>>>>>>> image >>>>>>>> maps for the relevant use cases in general. Is there existing >>>>>>>> precedent for >>>>>>>> supporting a navigation-related feature on <a> but not <area>? >>>>>>>> >>>>>>>> I don't have the answer to that - perhaps someone else will know. >>>>>>>> >>>>>>>> >>>>>>>> Given that we support this on multiple other navigation surfaces >>>>>>>> (<a>, window.open, and context-menu on <a>), and that the fix is >>>>>>>> quite simple >>>>>>>> <https://chromium-review.googlesource.com/c/chromium/src/+/6022268>, >>>>>>>> I'd err on the side of not breaking anyone, but we could also try to >>>>>>>> gather >>>>>>>> usage data first. >>>>>>>> >>>>>>>> Yes, agree - we should take a look at usage/potential breakage >>>>>>>> here. Have you tried to look at HTTPArchive? This feature has shipped >>>>>>>> long >>>>>>>> enough that there should be something there (if anything exists at >>>>>>>> all). Or >>>>>>>> there's the regular UMA route, but that's slower. >>>>>>>> >>>>>>> It would surprise me if this data showed up in HTTPArchive -- the >>>>>> majority of attributionsrc uses will be in dynamically injected DOM >>>>>> elements. >>>>>> >>>>>> We could try to go with UMA, but it may not be worth the effort. >>>>>> >>>>> >>>>> After recent breakage (caused by me) there's a desire to add >>>>> UseCounter or other lightweight UMA tracking before changing web-facing >>>>> behavior, particularly when there is otherwise no real way of knowing if >>>>> anyone is relying on it. The lack of developer signals increases the risk, >>>>> as does the very long time the existing behavior has been in place. >>>>> >>>>> I do strongly agree that the feature is worth doing. >>>>> >>>>> Stephen. >>>>> >>>> >>>> I'm not sure I understand what we would be measuring? As I understand >>>> it, this intent is to expose attributionsrc on <area> where previously it >>>> wasn't exposed, although it was already processed. It doesn't sound any >>>> different from typical "new feature" intents, in that we don't usually >>>> check if names, for example, conflict with existing code. If anything, this >>>> seems even safer in that the actual behavior wouldn't change (due to a >>>> current Chromium bug). >>>> >>>> Thanks, >>>> Vlad >>>> >>> >>> I made the comment about getting usage data because the conversation so >>> far seems to be indicating some chance of breakage. >>> >>> I guess to get breaking behavior you would need to be trying to set >>> element.attributeSrc in JS on an <area> element and having it fail now, >>> while it would start to work once this change is made. Is that right? >>> >>> In which case I don't think there is any way to measure that ahead of >>> time. It's also hard to see how a site would depend on the call failing. >>> But you could add a use counter to the CL and see if there is any immediate >>> usage at all, as anything other than near-zero initial usage suggests the >>> JS code was already in place somewhere. >>> >>> Or is there some other path to breakage? >>> >> We can add a usage counter, but I'm not clear on how this change could >> cause any breakage: Even setting element.attributionSrc in JS today on an >> <area> element wouldn't throw an exception or otherwise break; it would >> just create a new property on the element that does nothing. >> >>> >>> Stephen. >>> >>> >>>> >>>>> >>>>>> Blink component Blink >>>>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink> >>>>>>>>> >>>>>>>>> TAG review Covered by existing Attribution Reporting I2S as this >>>>>>>>> is a small change re-using the existing API surface. >>>>>>>>> >>>>>>>>> TAG review status Not applicable >>>>>>>>> >>>>>>>>> Risks >>>>>>>>> >>>>>>>>> >>>>>>>>> Interoperability and Compatibility >>>>>>>>> >>>>>>>>> None >>>>>>>>> >>>>>>>>> >>>>>>>>> *Gecko*: No signal >>>>>>>>> >>>>>>>>> *WebKit*: No signal >>>>>>>>> >>>>>>>>> *Web developers*: No signals >>>>>>>>> >>>>>>>>> *Other signals*: >>>>>>>>> >>>>>>>>> WebView application risks >>>>>>>>> >>>>>>>>> Does this intent deprecate or change behavior of existing APIs, >>>>>>>>> such that it has potentially high risk for Android WebView-based >>>>>>>>> applications? >>>>>>>>> >>>>>>>>> No. >>>>>>>>> >>>>>>>>> >>>>>>>>> Debuggability >>>>>>>>> >>>>>>>>> None >>>>>>>>> >>>>>>>>> >>>>>>>>> Will this feature be supported on all six Blink platforms >>>>>>>>> (Windows, Mac, Linux, ChromeOS, Android, and Android WebView)? >>>>>>>>> Yes >>>>>>>>> >>>>>>>>> Is this feature fully tested by web-platform-tests >>>>>>>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md> >>>>>>>>> ? Yes >>>>>>>>> >>>>>>>>> Flag name on about://flags None >>>>>>>>> >>>>>>>>> Finch feature name None >>>>>>>>> >>>>>>>>> Non-finch justification >>>>>>>>> >>>>>>>>> This is a minor change largely reusing existing code and behavior. >>>>>>>>> The only web-exposed detail here is the addition of an >>>>>>>>> already-processed >>>>>>>>> HTML attribute to the corresponding tag's IDL definition. >>>>>>>>> >>>>>>>>> >>>>>>>>> Requires code in //chrome? False >>>>>>>>> >>>>>>>>> Tracking bug https://issues.chromium.org/379275911 >>>>>>>>> >>>>>>>>> Measurement n/a >>>>>>>>> >>>>>>>>> Availability expectation Covered by existing Attribution >>>>>>>>> Reporting I2S as this is a small change re-using the existing API >>>>>>>>> surface >>>>>>>>> >>>>>>>>> Adoption expectation Covered by existing Attribution Reporting >>>>>>>>> I2S as this is a small change re-using the existing API surface >>>>>>>>> >>>>>>>>> Adoption plan n/a >>>>>>>>> >>>>>>>>> Non-OSS dependencies >>>>>>>>> >>>>>>>>> Does the feature depend on any code or APIs outside the Chromium >>>>>>>>> open source repository and its open-source dependencies to function? >>>>>>>>> No. >>>>>>>>> >>>>>>>>> Estimated milestones >>>>>>>>> Shipping on desktop 133 >>>>>>>>> Shipping on Android 133 >>>>>>>>> Shipping on WebView 133 >>>>>>>>> >>>>>>>>> Anticipated spec changes >>>>>>>>> >>>>>>>>> Open questions about a feature may be a source of future web >>>>>>>>> compat or interop issues. Please list open issues (e.g. links to known >>>>>>>>> github issues in the project for the feature specification) whose >>>>>>>>> resolution may introduce web compat/interop risk (e.g., changing to >>>>>>>>> naming >>>>>>>>> or structure of the API in a non-backward-compatible way). >>>>>>>>> n/a >>>>>>>>> >>>>>>>>> Link to entry on the Chrome Platform Status >>>>>>>>> https://chromestatus.com/feature/6547509428879360?gate=6545976813420544 >>>>>>>>> >>>>>>>>> This intent message was generated by Chrome Platform Status >>>>>>>>> <https://chromestatus.com>. >>>>>>>>> -- >>>>>>>>> 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 visit >>>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/673f72a6.2b0a0220.3bb1d2.02f2.GAE%40google.com >>>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/673f72a6.2b0a0220.3bb1d2.02f2.GAE%40google.com?utm_medium=email&utm_source=footer> >>>>>>>>> . >>>>>>>>> >>>>>>>>> -- >>>>>>>> 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 visit >>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/5f406b3c-98f1-4f62-94e9-43e61bba4556%40chromium.org >>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/5f406b3c-98f1-4f62-94e9-43e61bba4556%40chromium.org?utm_medium=email&utm_source=footer> >>>>>>>> . >>>>>>>> >>>>>>> -- >>>>>> 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 visit >>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP6jJUgkRFLr%3DP5FumrCoOh1bFembn6FqASLcm4BtZ5Vg4b7rw%40mail.gmail.com >>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP6jJUgkRFLr%3DP5FumrCoOh1bFembn6FqASLcm4BtZ5Vg4b7rw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>> . >>>>>> >>>>> -- >>>>> 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 visit >>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGsbWzQispi5Y8N7oWLjhS26U5p3TRs7HbZXcfjGyQg17Mgkfw%40mail.gmail.com >>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGsbWzQispi5Y8N7oWLjhS26U5p3TRs7HbZXcfjGyQg17Mgkfw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- >> 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 visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP6jJUhuAYmCTDQXSNmC9uH4KGYeuN9DnMyYWTiQiRsFsnh1Zg%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAP6jJUhuAYmCTDQXSNmC9uH4KGYeuN9DnMyYWTiQiRsFsnh1Zg%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> -- > 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 visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGsbWzT_-KK_JG_d0RjoBM8qkKbFzaceaEh7nqa6Zwdb3hZUow%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGsbWzT_-KK_JG_d0RjoBM8qkKbFzaceaEh7nqa6Zwdb3hZUow%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- 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 visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY9y9kjnXy1iZDhmew8FnqTUyeYoYqrFXTUL_0-7BE%2BTUQ%40mail.gmail.com.
