Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMake iframes block the enclosing document's load event #6677
Conversation
hoppipolla-critic-bot
commented
Jul 20, 2015
|
Critic review: https://critic.hoppipolla.co.uk/r/5608 This is an external review system which you may optionally use for the code review of your pull request. In order to help critic track your changes, please do not make in-place history rewrites (e.g. via |
|
@bors-servo r+ |
|
|
Make iframes block the enclosing document's load event It occurs to me as I write this that this doesn't handle the case of removing the iframe from the document before it's finished loading. Consider this an early feedback release! <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6677) <!-- Reviewable:end -->
|
|
|
|
|
|
Interestingly, the test fails because while the iframe borders disappear, the iframe content remains. This is unaffected by the presence of |
|
@jdm This means that the owning document has correctly hidden the iframe node, but that the compositor is still drawing the iframe layer, if that's of any help. |
|
r? @Ms2ger |
|
I think I'd like to finish the missing bits of unblocking the page if the iframe is removed before merging this again. |
|
With regards to the |
jdm
force-pushed the
jdm:iframeblockonload
branch
2 times, most recently
from
c43eccc
to
b46b91b
|
With regards to the test, we only get two load events. |
In cases where a pending parser-blocking script is executing, if the script caused another load to complete (eg. by removing a pending iframe from the DOM, thus terminating the load) then the currently-executing script would attempt to execute itself again.
…ical failure case.
|
@bors-servo: r=Ms2ger |
|
|
Make iframes block the enclosing document's load event It occurs to me as I write this that this doesn't handle the case of removing the iframe from the document before it's finished loading. Consider this an early feedback release! <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6677) <!-- Reviewable:end -->
|
|
|
@bors-servo: retry
|
|
|
|
|
jdm commentedJul 20, 2015
It occurs to me as I write this that this doesn't handle the case of removing the iframe from the document before it's finished loading. Consider this an early feedback release!