Bug 118285 - ToggleBold does not work in specific editable contents
Summary: ToggleBold does not work in specific editable contents
Status: RESOLVED DUPLICATE of bug 118185
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: KyungTae Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-02 01:30 PDT by KyungTae Kim
Modified: 2013-08-01 20:03 PDT (History)
2 users (show)

See Also:


Attachments
toggleBold test case (135 bytes, text/html)
2013-07-02 01:30 PDT, KyungTae Kim
no flags Details
Patch (1.61 KB, patch)
2013-07-02 01:42 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (5.57 KB, patch)
2013-07-02 21:25 PDT, KyungTae Kim
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (510.84 KB, application/zip)
2013-07-03 01:05 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description KyungTae Kim 2013-07-02 01:30:41 PDT
Created attachment 205877 [details]
toggleBold test case

ToggleBold does not work in specific editable contents.
If you open the attached test case and try to toggle bold 2 times, 
you can find the text isn't changed from 'bold' to 'normal'.

It seems if there is an anonymous block that doesn't have text as a first child,
the current style is checked as "mixed" even though all the visible text is bold.
Comment 1 KyungTae Kim 2013-07-02 01:42:22 PDT
Created attachment 205879 [details]
Patch
Comment 2 KyungTae Kim 2013-07-02 02:56:32 PDT
In the test case, the below starred element("\n") is the text node without renderer.
It need to be excluded when checking whether the selected area has "mixed style" or not.

BODY	0x105c990
	DIV	0x105f0b0
		#text	0xfe2790 "\n"
		DIV	0xfe27f0
			B	0xf6eb40
				#text	0x1051390 "\nSelect from here\n"
*		#text	0xfe2980 "\n"
		B	0x1053ec0
			IMG	0xfb1590
			#text	0x105b4c0 "\nto here and try to toggle bold(with Ctrl+B) 2 times"
		#text	0x105fb10 ".\n"
	#text	0x105fc00 "\n"
Comment 3 Ryosuke Niwa 2013-07-02 04:35:03 PDT
Thanks for filing a bug and posting a patch. But please edit the bug summary to be more descriptive. Bolding doesn't work in "specific editable contents" is a very vague description.
Comment 4 Ryosuke Niwa 2013-07-02 04:39:59 PDT
Comment on attachment 205879 [details]
Patch

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

r- due to a lack of regression tests and inadequate change log description.

> Source/WebCore/ChangeLog:8
> +        Check only the text that are visible when checking triStateOfStyle

This doesn't explain why we're making this code change.

A good change log entry describes what is causing the bug and how the particular code change fixes it.
See http://www.webkit.org/coding/contributing.html for an example.

> Source/WebCore/ChangeLog:9
> +

We also need to add a test. You can automate bolding action via execCommand.
I also suspect you need to force Windows editing behavior via internals.setEditingBehavior.
Comment 5 KyungTae Kim 2013-07-02 21:25:04 PDT
Created attachment 205965 [details]
Patch
Comment 6 Build Bot 2013-07-03 01:05:05 PDT
Comment on attachment 205965 [details]
Patch

Attachment 205965 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1012857

New failing tests:
editing/style/5065910.html
Comment 7 Build Bot 2013-07-03 01:05:07 PDT
Created attachment 205974 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 8 Ryosuke Niwa 2013-08-01 20:02:47 PDT

*** This bug has been marked as a duplicate of bug 118185 ***
Comment 9 Ryosuke Niwa 2013-08-01 20:03:06 PDT
Comment on attachment 205965 [details]
Patch

This bug has been fixed in http://trac.webkit.org/changeset/153509.