Use tighter typing in editing: FormatBlockCommand / FrameSelection

Use tighter typing in editing: FormatBlockCommand / FrameSelection

Use tighter typing in editing/ to increase code readability. This CL focuses
on FormatBlockCommand / FrameSelection.

R=leviw@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179170

[Inactive, 2014-07-26 19:40:22]

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1819: if (HTMLBodyElement* body =
Traversal<HTMLBodyElement>::firstChild(*documentElement))
See Document::body() at
https://chromium.googlesource.com/chromium/blink/+/master/Source/core/dom/Doc...

We only need to look at direct children of the document element (html element of
HTML documents). Document::body() also looks for HTMLFrameSetElement in addition
to HTMLBodyElement so we cannot simply call Document::body() here (without
changing the behavior).

[eseidel, 2014-07-26 21:16:55]

Yosin is a great person for editing reviews.  Surprised this didn't auto-cc more
people.  Maybe Yosin hasn't set up an editing watchlist yet.

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1819: if (HTMLBodyElement* body =
Traversal<HTMLBodyElement>::firstChild(*documentElement))
On 2014/07/26 19:40:22, Chris Dumez wrote:
> See Document::body() at
>
https://chromium.googlesource.com/chromium/blink/+/master/Source/core/dom/Doc...
> 
> We only need to look at direct children of the document element (html element
of
> HTML documents). Document::body() also looks for HTMLFrameSetElement in
addition
> to HTMLBodyElement so we cannot simply call Document::body() here (without
> changing the behavior).

I guess non-html documents are going the way of the dodo.   This code doesn't
handle <svg> roots, which is probably fine.

[Inactive, 2014-07-26 21:44:54]

On 2014/07/26 21:16:55, eseidel wrote:
> Yosin is a great person for editing reviews.  Surprised this didn't auto-cc
more
> people.  Maybe Yosin hasn't set up an editing watchlist yet.

Ok, thanks. I'll make sure to add Yosin as reviewer on future CLs. Levi has been
very responsive so far :)

[yosin_UTC9, 2014-07-29 04:38:46]

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1819: if (HTMLBodyElement* body =
Traversal<HTMLBodyElement>::firstChild(*documentElement))
I think purpose of this patch is improving typing. So, it should be

HTMLBodyElement* body = isHTMLBodyElemnt(*node) ?
static_Cast<HTMLBodyElement*>(node) : Traversal<HTMLBodyElement>::next(*node);

I hope another patch will make to use Document::body().

[Inactive, 2014-07-29 12:01:00]

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
File Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/420103002/diff/1/Source/core/editing/FrameSel...
Source/core/editing/FrameSelection.cpp:1819: if (HTMLBodyElement* body =
Traversal<HTMLBodyElement>::firstChild(*documentElement))
On 2014/07/29 04:38:46, Yosi_UTC9 wrote:
> I think purpose of this patch is improving typing. So, it should be
> 
> HTMLBodyElement* body = isHTMLBodyElemnt(*node) ?
> static_Cast<HTMLBodyElement*>(node) : Traversal<HTMLBodyElement>::next(*node);
> 
> I hope another patch will make to use Document::body().

Ok, I'll do this in a separate patch. It just felt wrong to update the typing on
that line but leave the bad logic as is. Anyway, please take a look at the
latest patchset.

[leviw_travelin_and_unemployed, 2014-07-29 18:25:37]

lgtm

[Inactive, 2014-07-29 18:38:49]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-07-29 18:39:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/420103002/20001

[commit-bot: I haz the power, 2014-07-29 19:02:51]

Change committed as 179170