Bug 70248 – Pass a Frame* parameter in EditorClient::respondToChangedSelection

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Antonio Gomes]

Most of the port specific implementations of EditorClient::respondToChangedSelection (like EditorClient{Qt,Gtk,etc}) are wrongly relying on FocusController::focusedOrMainFrame method to get the Frame where the selection is happening on.

It is not right, since a selection can be easily triggered in an inner frame that is not focused.

Patch coming...

[Antonio Gomes]

Created attachment 111284 [details]
patch v1 - for EWS to build

[WebKit Review Bot]

Attachment 111284 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
Current branch master is up to date.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antonio Gomes]

cc'ed some port peers to be aware of the patch (and maybe review port specific parts effected).

[Gyuyoung Kim]

Comment on attachment 111284 [details]
patch v1 - for EWS to build

Attachment 111284 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10085426

[Antonio Gomes]

(In reply to comment #4)
> (From update of attachment 111284 [details])
> Attachment 111284 [details] did not pass efl-ews (efl):
> Output: http://queues.webkit.org/results/10085426

bleh, obvious errors :/

[Antonio Gomes]

Created attachment 111296 [details]
patch v1.1 - for EWS to build

[WebKit Review Bot]

Attachment 111296 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
Current branch master is up to date.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 111296 [details]
patch v1.1 - for EWS to build

Attachment 111296 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10098355

[Antonio Gomes]

Created attachment 111297 [details]
patch v1.2 - for EWS to build

[WebKit Review Bot]

Attachment 111297 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
Current branch master is up to date.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 111297 [details]
patch v1.2 - for EWS to build

Attachment 111297 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10086572

[Antonio Gomes]

(In reply to comment #10)
> Attachment 111297 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2
> 
> Updating OpenSource
> Current branch master is up to date.
> Updating chromium port dependencies using gclient...
> Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
> Re-trying 'depot_tools/gclient sync'
> Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
> Re-trying 'depot_tools/gclient sync'
> Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
> Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
> Re-trying 'depot_tools/gclient sync'
> No such file or directory at Tools/Scripts/update-webkit line 104.
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

Ignore this.


(In reply to comment #11)
> (From update of attachment 111297 [details])
> Attachment 111297 [details] did not pass efl-ews (efl):
> Output: http://queues.webkit.org/results/10086572

And this :) it is bug 70253

[Darin Adler]

Comment on attachment 111297 [details]
patch v1.2 - for EWS to build

This is wrong. The editor client is per-frame and so can store a Frame* if needed. It’s not a per-Page thing.

[Darin Adler]

Comment on attachment 111297 [details]
patch v1.2 - for EWS to build

Oops, my bad, it is per-Page, not per-Frame!

[Ryosuke Niwa]

Comment on attachment 111297 [details]
patch v1.2 - for EWS to build

View in context: https://bugs.webkit.org/attachment.cgi?id=111297&action=review

> Source/WebCore/ChangeLog:4
> +

Missing bug url.

> Source/WebCore/ChangeLog:19
> +        * editing/Editor.cpp: Pass the Frame on where the selection is
> +		changing to the client.

It seems like this can fit in on line.

[Antonio Gomes]

Created attachment 111314 [details]
patch v1.3 - for EWS to build

[WebKit Review Bot]

Attachment 111314 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
Current branch master is up to date.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 111314 [details]
patch v1.3 - for EWS to build

Attachment 111314 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10106277

[Antonio Gomes]

Created attachment 111372 [details]
patch v1.4 - for review

[Antonio Gomes]

Created attachment 111439 [details]
patch 1.5

[Ryosuke Niwa]

Comment on attachment 111439 [details]
patch 1.5

Hopefully it won't break any tests on any port.

[Gustavo Noronha (kov)]

Comment on attachment 111439 [details]
patch 1.5

Attachment 111439 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10138094

[Antonio Gomes]

Created attachment 111553 [details]
patch 1.6

[Antonio Gomes]

Created attachment 111610 [details]
(landed as r100874) - patch 1.7 - for EWS (r=rniwa)- do not review

[Antonio Gomes]

Comment on attachment 111610 [details]
(landed as r100874) - patch 1.7 - for EWS (r=rniwa)- do not review

landing it manually, just waiting for a green state on the bots

[Antonio Gomes]

Comment on attachment 111610 [details]
(landed as r100874) - patch 1.7 - for EWS (r=rniwa)- do not review

Clearing flags: r100874

> git svn dcommit 
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/editing/Editor.cpp
	M	Source/WebCore/loader/EmptyClients.h
	M	Source/WebCore/page/EditorClient.h
	M	Source/WebKit/chromium/ChangeLog
	M	Source/WebKit/chromium/src/EditorClientImpl.cpp
	M	Source/WebKit/chromium/src/EditorClientImpl.h
	M	Source/WebKit/efl/ChangeLog
	M	Source/WebKit/efl/WebCoreSupport/EditorClientEfl.cpp
	M	Source/WebKit/efl/WebCoreSupport/EditorClientEfl.h
	M	Source/WebKit/gtk/ChangeLog
	M	Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp
	M	Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.h
	M	Source/WebKit/mac/ChangeLog
	M	Source/WebKit/mac/WebCoreSupport/WebEditorClient.h
	M	Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
	M	Source/WebKit/qt/ChangeLog
	M	Source/WebKit/qt/WebCoreSupport/EditorClientQt.cpp
	M	Source/WebKit/qt/WebCoreSupport/EditorClientQt.h
	M	Source/WebKit/win/ChangeLog
	M	Source/WebKit/win/WebCoreSupport/WebEditorClient.cpp
	M	Source/WebKit/win/WebCoreSupport/WebEditorClient.h
	M	Source/WebKit/wx/ChangeLog
	M	Source/WebKit/wx/WebKitSupport/EditorClientWx.cpp
	M	Source/WebKit/wx/WebKitSupport/EditorClientWx.h
	M	Source/WebKit2/ChangeLog
	M	Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp
	M	Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h
Committed r100874
	M	Source/WebKit/chromium/ChangeLog
	M	Source/WebKit/chromium/src/EditorClientImpl.cpp
	M	Source/WebKit/chromium/src/EditorClientImpl.h
	M	Source/WebKit/qt/WebCoreSupport/EditorClientQt.h
	M	Source/WebKit/qt/WebCoreSupport/EditorClientQt.cpp
	M	Source/WebKit/qt/ChangeLog
	M	Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.h
	M	Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp
	M	Source/WebKit/gtk/ChangeLog
	M	Source/WebKit/win/ChangeLog
	M	Source/WebKit/win/WebCoreSupport/WebEditorClient.cpp
	M	Source/WebKit/win/WebCoreSupport/WebEditorClient.h
	M	Source/WebKit/wx/WebKitSupport/EditorClientWx.cpp
	M	Source/WebKit/wx/WebKitSupport/EditorClientWx.h
	M	Source/WebKit/wx/ChangeLog
	M	Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
	M	Source/WebKit/mac/WebCoreSupport/WebEditorClient.h
	M	Source/WebKit/mac/ChangeLog
	M	Source/WebKit/efl/WebCoreSupport/EditorClientEfl.cpp
	M	Source/WebKit/efl/WebCoreSupport/EditorClientEfl.h
	M	Source/WebKit/efl/ChangeLog
	M	Source/WebCore/editing/Editor.cpp
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/page/EditorClient.h
	M	Source/WebCore/loader/EmptyClients.h
	M	Source/WebKit2/ChangeLog
	M	Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp
	M	Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h
r100874 = fdc7310332b67a164b63d03c3412604b5ea7a296 (refs/remotes/trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk