Implement EOF handling in TextMode
Created attachment 67382 [details] Patch
Comment on attachment 67382 [details] Patch This patch breaks the old versions of Apple’s Mail application. There are a conditional comments here such as “if we need this setting”, but we do need it. I know it’s easier to just remove it, but we at Apple need to keep this. Maybe you can find someone from my team at Apple to work with you on this.
@abarth: you should just split this up so the text stuff can be landed w/o contention. Also, I don't think the mail-specific code works anymore. But someone would need to test.
Comment on attachment 67382 [details] Patch Oh, nm. I see these are tied. I guess I agree then. We should remove this code and add it back with tests. It's possible to test this now that we have layoutTestController.overridePreference(). We can use that in conjunction with a platform/mac-leopard or mac-tiger test to test this code if it's truly needed.
"if it's truly needed" -> it's possible that the new parser behavior matches old-mails needed quirk already. Since the old code isn't tested, its impossible to know.
(In reply to comment #3) > But someone would need to test. Whether this patch broke it or the HTML parser work broke it earlier, this does need to be kept working. It doesn’t need to be tracked in this bug report. Here’s how someone can test it: On Leopard, run Apple Mail with the WebKit you wish to test. Open Mail’s Preferences. Click on Signatures. Click the "+" once. This should correctly make a user signature. Click the "+" again. If the bug is active, no signature is created and exception will occur.
The incorrect old mail assumption was that the first child of the first child of the document is the body, not the head.
> This patch breaks the old versions of Apple’s Mail application. There are a conditional comments here such as “if we need this setting”, but we do need it. I know it’s easier to just remove it, but we at Apple need to keep this. Maybe you can find someone from my team at Apple to work with you on this. I think this setting is already broken. This patch only affects the behavior of EOF in "text mode," which is if the document ends before we close a <style> or <script> tag. Who's a good person to talk to about the requirements of Mail? I can guess from how the old code worked, but it would be best to gather the requirements directly instead of reverse engineering them from the previous implementation.
(In reply to comment #7) > The incorrect old mail assumption was that the first child of the first child of the document is the body, not the head. I see. Presumably in the case where the head is created implicitly and there are no XML processing directives / DOCTYPEs / etc.
(In reply to comment #9) > (In reply to comment #7) > > The incorrect old mail assumption was that the first child of the first child of the document is the body, not the head. > > I see. Presumably in the case where the head is created implicitly and there are no XML processing directives / DOCTYPEs / etc. That’s right. And it’s only the older versions of Mail that have this dependency.
Created attachment 67431 [details] Patch for landing
@darin: It seems a better hack for this would be to just hack the Obj-C DOM bindings layer. Make a DOMHTMLDocumentCustom.mm file and have it check the bundle identifier for Mail and override firstChild to return body instead of the real first child. Hacking this into WebCore seems like a bad idea.
The hack would involve hacking Node.idl to make a custom binding for firstChild in the Leopard/Tiger Obj-C case. This is assuming of course that this is how Mail is accessing the first child of the document and assuming it's the body. :)
It’s OK to put this hack in the bindings layer, but I’m not sure it’s better. It’s probably more complicated, not less.
(In reply to comment #14) > It’s OK to put this hack in the bindings layer, but I’m not sure it’s better. It’s probably more complicated, not less. The version in the parser is https://bugs.webkit.org/show_bug.cgi?id=45693
Comment on attachment 67431 [details] Patch for landing Clearing flags on attachment: 67431 Committed r67406: <http://trac.webkit.org/changeset/67406>
All reviewed patches have been landed. Closing bug.