LGTM3 On Wednesday, December 4, 2024 at 8:08:09 AM UTC-8 Rick Byers wrote:
> 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/0b8e0b8f-371c-4158-8526-f2ce4c0b8e38n%40chromium.org.
