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

containerSrc could be long (a data URI) #64

Open
tdresser opened this issue Jun 12, 2019 · 16 comments
Open

containerSrc could be long (a data URI) #64

tdresser opened this issue Jun 12, 2019 · 16 comments
Assignees
Milestone

Comments

@tdresser
Copy link
Contributor

We should likely truncate containerSrc in cases where it's too long.

@domenic
Copy link
Contributor

domenic commented Jun 12, 2019

We don't do this for anything else on the platform that exposes a URL... what's the rationale?

@tdresser
Copy link
Contributor Author

If you completely delete an element, it's associated PerformanceEntry could keep the full data URI alive for no good reason. I believe this is different from other ways URLs are exposed today.

This may just be premature optimization though.

@yoavweiss @rniwa @toddreifsteck, any thoughts?

@toddreifsteck
Copy link
Member

My thought is that this is premature optimization. @rniwa ?

@hawkinsw
Copy link
Contributor

If you completely delete an element, it's associated PerformanceEntry could keep the full data URI alive for no good reason. I believe this is different from other ways URLs are exposed today.

Can you explain to me why the PerformanceEntry would stay alive for an element that is completely deleted? I am sorry to be stupid.

This may just be premature optimization though.

@yoavweiss @rniwa @toddreifsteck, any thoughts?

@rniwa
Copy link

rniwa commented Jul 1, 2019

I mean... there is no mechanism for PerformanceEntry to removed for long tasks right now, right?

@hawkinsw
Copy link
Contributor

hawkinsw commented Jul 1, 2019 via email

@toddreifsteck
Copy link
Member

The PerformanceEntry is enqueued to Performanee Timeline. Is the underlying question whether the Performance Timeline can delete the entry if there is no buffering behavior specified?

@rniwa
Copy link

rniwa commented Jul 1, 2019

At this point then, what we are saying is that an implementer might choose to keep it around because there is nothing in the spec that mandates its deletion

It's not so much that an implementation may choose to keep it in the timeline. The specifications mandates that whatever entry added to the timeline remain in the timeline unless the buffering limit is reached, or it's explicitly cleared. Whether the associated element is removed from the document is irrelevant in this regard.

@hawkinsw
Copy link
Contributor

hawkinsw commented Jul 1, 2019 via email

@yoavweiss
Copy link
Contributor

The PerformanceEntry is enqueued to Performance Timeline. Is the underlying question whether the Performance Timeline can delete the entry if there is no buffering behavior specified?

IIRC, currently for LongTasks, they are only queued for a PerformanceObserver, which means that a site that ignores those entries will not keep that URL alive. We are talking about keeping them in the timeline in the future though (through explicit registration).

It may make sense to explicitly set apart data URIs from e.g. HTTP schemes.

It's also worth noting that at least in some implementations, those URLs can take up that space exactly once, even if multiple LongTask entries point at them.

I do wonder, though, why we did not address removing Performance Entries from the Performance Timeline if/when their associated elements are deleted from the document.

IMO, it doesn't really make sense to remove such entries from the timeline, as they did happen (and impacted the user). Later removal of the element doesn't change that.

@npm1
Copy link
Contributor

npm1 commented Jul 2, 2019

Yes, right now there is no buffered flag behavior nor performance timeline buffering for longtask entries, so they would only need to be kept alive if a PerformanceObserver looking at them stores them in JS. Is it common to have long inline scripts? Exposing the full URI does not seem to be flagged to Chrome as a memory issue thus far. Maybe it could become one if we support buffering longtask entries in the future.

@toddreifsteck toddreifsteck added this to the Level 1 milestone Aug 1, 2019
@toddreifsteck
Copy link
Member

Per call on 8/1: Consider an arbitrary limit for data uri's universally for all Performance Timeline exposed PerformanceEntry's.

@yoavweiss
Copy link
Contributor

@npm1 - I know that we're applying limits on URL length elsewhere. Would be good to consolidate our approach.

@npm1
Copy link
Contributor

npm1 commented Jan 10, 2022

Yes, although for different resource types. Consolidating still seems reasonable to me.

@npm1
Copy link
Contributor

npm1 commented Jan 24, 2022

Similar issue: w3c/element-timing#64

@noamr noamr self-assigned this Jan 16, 2023
@noamr
Copy link
Contributor

noamr commented Jan 16, 2023

I wonder if we should follow this guideline, and leave this limitation implementation-defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants