Closed
Bug 559970
Opened 15 years ago
Closed 15 years ago
inputs that create textboxes should allow lazy frame construction
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
(Keywords: perf)
Attachments
(2 files, 1 obsolete file)
|
3.74 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
|
2.36 KB,
patch
|
Details | Diff | Splinter Review |
Currently input elements that create textboxes do not get their frames constructed lazily due to a check to get around editor bugs that flags them for immediate construction. But those editor bugs don't affect this situation, so we should construct them lazily. See bug 473178 comments 14-20.
|
Assignee | |
Comment 2•15 years ago
|
||
Refine the editability condition in MaybeConstructLazily so that inputs are constructed lazily
Assignee: nobody → tnikkel
Attachment #440681 -
Flags: review?(bzbarsky)
|
||
Updated•15 years ago
|
Summary: input's that create textboxes should allow lazy frame construction → inputs that create textboxes should allow lazy frame construction
|
|
||
Comment 3•15 years ago
|
||
Comment on attachment 440681 [details] [diff] [review]
patch
r=bzbarsky
Attachment #440681 -
Flags: review?(bzbarsky) → review+
|
|
Assignee | |
Comment 4•15 years ago
|
||
In browser_463205_sample.html we have
<body>
<iframe src="data:text/html,<input%20id='original'>"></iframe>
<iframe src="browser_463205_helper.html"></iframe>
<iframe src="data:text/html,mark1"></iframe>
<script type="application/javascript">
frames[2].addEventListener("DOMContentLoaded", function() {
frames[2].removeEventListener("DOMContentLoaded", arguments.callee, false);
if (frames[2].document.location.href == "data:text/html,mark1") {
frames[2].document.location = "data:text/html,mark2";
}
else {
frames[1].document.location.hash = "#original";
frames[0].document.location = "http://mochi.test:8888/browser/" +
"browser/components/sessionstore/test/browser/browser_463205_helper.html";
}
}, false);
</script>
</body>
The DOMContentLoaded event for the 3rd iframe races with the load event for the 1st iframe. Without lazy frame construction for textboxes the load event for the 1st iframe comes first, then the DOMContentLoaded event for the third iframe fires and changes the location of the 1st iframe, so we get a second load coming from the 1st iframe.
With lazy frame construction for textboxes the DOMContentLoaded event for the third iframe comes before the load event for the 1st iframe when it is still loading "data:text/html,<input%20id='original'>", so the load event from the 1st iframe for the page "data:text/html,<input%20id='original'>" never fires because we change the location of the 1st iframe before it does. This causes us to get one less load event. I changed the test to be less fragile by listening for the load event for the last page we expect for each iframe.
|
|
Assignee | |
Comment 5•15 years ago
|
||
This patch takes a different approach, it makes the changing of the locations for the first two iframes wait until the third iframe gets DOMContentLoaded _and_ the first two iframes get load. This seems like it will still test the same things. I've emailed the original testcase author from bug 463205 as well as the person who fixed the bug to get some kind of confirmation, but I don't know if I'll get a reply.
Attachment #440692 -
Attachment is obsolete: true
|
||
Comment 6•15 years ago
|
||
Comment on attachment 441090 [details] [diff] [review]
Fix a test better.
The idea of the test was to reload the first iframe before the whole frameset has loaded (IOW: ASAP).
In that spirit, I'd rather just count the load events until the third iframe's DOMContentLoaded and then dispatch the missing load events yourself.
If that doesn't work, I'd rather listen for the load event in the frameset and dispatch a different event (e.g. a MessageEvent) when the first frame (re)loads and listen for that other event in the test (instead of counting load events).
|
|
Assignee | |
Comment 7•15 years ago
|
||
Thanks for commenting.
What about the approach taken by attachment 440692 [details] [diff] [review], where we watch for the load events for the final location we expect in each iframe?
|
|
||
Comment 8•15 years ago
|
||
(In reply to comment #7)
That should work as well, you might even get the same result with less code:
> function hasLoaded(aFrame) aFrame.document.readyState == "complete";
> if (!Array.prototype.every.call(content.frames, hasLoaded))
> return;
|
|
Assignee | |
Comment 9•15 years ago
|
||
I think I'll stick with what I have because we are reloading two iframes there is a remote possibility that all frames are loaded but have not yet loaded the final page.
Thanks.
|
|
Assignee | |
Comment 10•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/20dc1a1dc2d9
http://hg.mozilla.org/mozilla-central/rev/d58ccc4c5f35
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•