Timothy told me that he's willing to investigate what kinds of stuff fail in the editor world which caused him to disable lazy frame construction in editable content before. This can help with editing performance in web apps, Orion included.

Try run for 728387aa2de7 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=728387aa2de7 Results (out of 32 total builds): success: 20 warnings: 12 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/tnikkel@gmail.com-728387aa2de7

So the IsEditable check that we use to disable lazy frame construction is only one check that might force eager frame construction for editable elements. We also disable lazy frame construction for native anonymous content (and in native anonymous subtrees). This is because there is no easy way to traverse native anonymous content when we are traversing the content tree looking for nodes that need frames constructed later.

That's fine for webapps, which are editing non-anonymous DOMs.

This is a *lot* of failures. :(

One or two patches might fix the majority of them. (Just a hunch, based on nothing.)

(In reply to Timothy Nikkel (:tn) from comment #5) > One or two patches might fix the majority of them. (Just a hunch, based on > nothing.) Do you feel like delving into this a bit, and filing some editor bugs for me to take over?

I think I'll pick one failure and investigate it more closely. That way if other failures are fixed by the same patch that fixes the first I don't waste time figuring out that they have the same root cause.

That sounds great, thanks!

I quickly was reminded of this code in nsEditor::IsEditable which is likely the source of a lot of problems: nsIFrame *resultFrame = content->GetPrimaryFrame(); if (!resultFrame) // if it has no frame, it is not editable return PR_FALSE;

The origin of that code is a 1999 commit whose commit message includes "IsEditable is much less hacky." I think determining what this function is intended to do, and changing it so that it doesn't rely on frames existing is a good first step.

Yeah, to the best of my knowledge, this function is supposed to make sure that the editor doesn't modify display:none elements. I wrote a patch today which does that without touching the frame tree or flushing. It is hitting a few failures in the editor/ tests. I debugged a lot of them, but there are still 10 or so left for me to debug. I'll file a bug and attach that patch there once I figure out those failures. That should be a great step here, since that function is called from all around the place.

My uneducated wild guess of just checking if the content node IsEditable() if it has no frame and returning true if it is fixes all but 12 of the failures: https://tbpl.mozilla.org/?tree=Try&rev=affaf8a41fe9 Just a data point, as I'm sure Ehsan's patch is much better.

(In reply to Timothy Nikkel (:tn) from comment #12) > My uneducated wild guess of just checking if the content node IsEditable() > if it has no frame and returning true if it is fixes all but 12 of the > failures: > https://tbpl.mozilla.org/?tree=Try&rev=affaf8a41fe9 > > Just a data point, as I'm sure Ehsan's patch is much better. OK, this should be fixed by my patch in bug 688789, which I will land soon. Timothy, can you please use that patch and see what other failures we're running into?

Try run for aac2cb281498 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=aac2cb281498 Results (out of 24 total builds): success: 20 warnings: 2 failure: 2 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/tnikkel@gmail.com-aac2cb281498

Only one failure left, pasting it here in case the try logs go away. 13957 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_htmleditor_keyevent_handling.html | non-tabbable HTML editor: Shift+Tab after Tab on TD wasn't prevented on bubbling phase - got false, expected true 14000 ERROR TEST-UNEXPECTED-FAIL | /tests/editor/libeditor/html/tests/test_htmleditor_keyevent_handling.html | non-tabbable HTML editor: Shift+Tab after Tab on TH wasn't prevented on bubbling phase - got false, expected true

With the extra flush I propose in bug 699703 to fix the last failure this goes green on try server.

(In reply to Timothy Nikkel (:tn) from comment #16) > With the extra flush I propose in bug 699703 to fix the last failure this > goes green on try server. Sorry, I had missed the bugmail here. Would you mind testing again with that proposed flush? If all is green, I think we should get this landed!

dup to new bug