Summary: | Webkit should generate "compositionstart", "compositionend", and "text" events like firefox | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Danilatos <daniel.danilatos> | ||||||||||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | ap, dimich, eric, hamaji, hbono, ojan, oliver | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
URL: | http://www.danilatos.com/event-test/ExperimentTest.html | ||||||||||||||||
Attachments: |
|
Description
Daniel Danilatos
2009-06-10 22:40:14 PDT
Created attachment 42203 [details]
a work-in-progress patch
Even though this change works good on my Macbook, I'm not sure it works on Windows or Linux.
(That is, this change is not ready for review.)
So, I just would like to hear opinions.
Regards,
Hironori Bono
Created attachment 42736 [details]
A proposed patch (only for Mac)
Sorry for my slow updates.
Even though I tried setting up build environments for WebKit GTK, WebKit QT, and WebKit Win, I wasn't able to set up build them.
So, I enclosed some part of my previous change with #if PLATFORM(MAC) and #endif to avoid possible build breaks occurred by the lack of the new files.
Is it possible to review this change and give me your comments?
Regards,
Hironori Bono
Created attachment 42744 [details]
A proposed patch (only for Mac)
Created attachment 42745 [details]
A proposed patch 2 (only for Mac)
Sorry. I attached a wrong patch. (The previous one may send a compositionstart event even when m_compositionNode == 0 && text.isEmpty(), when setComposition() doesn't create a composition node.)
Comment on attachment 42745 [details]
A proposed patch 2 (only for Mac)
What is the reason for having the code be Mac only? I can't see any mac specific code in it...
Your test case should describe how to run the test manually
--Oliver
Created attachment 42854 [details] A proposed patch 3 (for WebKit and Chromium) Thank you for your comments. > What is the reason for having the code be Mac only? I can't see any mac specific code in it... This is just because I haven't had any idea how I can add files to GTK (or QT) projects. I have added them and checked this change can be compiled on WebKit GTK and WebKit Win. (I'm not confident this is correct, though. Please feel free to blame me if I'm wrong.) > Your test case should describe how to run the test manually Thank you for noticing this. I added a brief comment how to use my layout test manually Regards, Hironori Bono Comment on attachment 42854 [details]
A proposed patch 3 (for WebKit and Chromium)
r=me
Comment on attachment 42854 [details]
A proposed patch 3 (for WebKit and Chromium)
Bono-san doesn't seem to be a committer yet, so adding to the cq.
Comment on attachment 42854 [details]
A proposed patch 3 (for WebKit and Chromium)
Rejecting patch 42854 from commit-queue.
Failed to run "WebKitTools/Scripts/build-webkit" exit_code: 1
Last 500 characters of output:
e/rendering/RenderRubyRun.cpp -o /Users/eseidel/Projects/build/WebCore.build/Release/WebCore.build/Objects-normal/i386/RenderRubyRun.o
** BUILD FAILED **
The following build commands failed:
WebCore:
Distributed-CompileC /Users/eseidel/Projects/build/WebCore.build/Release/WebCore.build/Objects-normal/i386/JSCompositionEvent.o /Users/eseidel/Projects/CommitQueue/WebCore/../WebKitBuild/Release/DerivedSources/WebCore/JSCompositionEvent.cpp normal i386 c++ com.apple.compilers.gcc.4_2
(1 failure)
Bono-san will need to investigate the build failure and post a new patch. Created attachment 43031 [details]
proposed patch 4 (for WebKit and Chromium)
Hi Eric,
Thank you so much for your review, and sorry for this build failure caused by my change.
I noticed the xcode project file in my previous change always refers to "WebKitBuild/Release/DerivedSources/WebCore/JSCompositionEvent.cpp" and it causes a build break when we build the debug version without building the release version. (This is a bonehead mistake of mine in adding JSCompositionEvent.{cpp,h} to this project.)
To fix this problem, I have changed the project file, and confirmed I can compile the debug version without building the release one.
(Except this change, this patch is same as the previous one.)
Would it be possible to review the update one?
Regards,
Hironori Bono
Comment on attachment 42854 [details]
A proposed patch 3 (for WebKit and Chromium)
Clean up patch from pending-commit.
Comment on attachment 43031 [details] proposed patch 4 (for WebKit and Chromium) The patch 43031 is identical to the one Oliver r+'ed, except the fixed path to JSCompositionEvent files in pbxproj. One more thing: > Index: WebCore/WebCore.vcproj/WebCore.vcproj > <File > + RelativePath="..\inspector\front-end\DOMStorageDataGrid.js" > + > > + </File> > + <File I think this is not an intended change. This file does not exist and seems not relevant to the patch. Since the rest of the change is already reviewed, I will try to fix up vcproj and land this manually. Comment on attachment 43031 [details]
proposed patch 4 (for WebKit and Chromium)
Removing r? since it doesn't need another review, just landing.
(In reply to comment #15) > Landed: http://trac.webkit.org/changeset/50968 Thank you so much for fixing my mistake in the vcproj file and land this change. |