Move mozilla::dom::Selection code from layout/generic/ to dom/base/

RESOLVED FIXED in Firefox 56

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

Trunk
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

2 years ago
We currently have code for mozilla::dom::Selection, nsFrameSelection and
mozilla::dom::SelectionChangeListener + various helper functions and
classes for those all living in layout/generic/nsSelection.cpp.
It's 7224 lines long and it's a mess.

Most of it implements DOM Selection and thus belongs under dom/base/.
So I intend to split up this file like so:

mozilla::dom::Selection code moves to a new file dom/base/Selection.cpp
and its associated header file moves from layout/generic/Selection.h
to dom/base/Selection.h

nsFrameSelection code moves to layout/generic/nsFrameSelection.cpp,
its header file layout/generic/nsFrameSelection.h stays as is

mozilla::dom::SelectionChangeListener code moves to a new file
dom/base/SelectionChangeListener.cpp, its header file
layout/generic/SelectionChangeListener.h moves to
dom/base/SelectionChangeListener.h

Does this plan make sense to you?
Assignee

Comment 4

2 years ago
I think that's a good first step to untangle this mess.  All of the above are just
verbatim code shuffling.  Next step would be to move some code between Selection.cpp
<-> nsFrameSelection.cpp to make the purpose of each clearer.  Any remaining event
handling should move to nsFrameSelection, and manipulation of selection ranges
should move to Selection.
Attachment #8878903 - Flags: review?(bugs) → review+
Attachment #8878904 - Flags: review?(bugs) → review+
Attachment #8878905 - Flags: review?(bugs) → review+

Comment 5

2 years ago
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5150df804e43
part 1 - Move layout/generic/nsSelection.cpp verbatim to dom/base/Selection.cpp, and layout/generic/Selection*.h to dom/base/.  Also export a few table header files that it needs.  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/f76616ab2eb0
part 2 - Create layout/generic/nsFrameSelection.cpp and move nsFrameSelection code from dom/base/Selection.cpp to it.  r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/733f3795e973
part 3 - Create dom/base/SelectionChangeListener.cpp and move mozilla::dom::SelectionChangeListener code from dom/base/Selection.cpp to it.  r=smaug
Oh, looks like that nsFrameSelection.cpp lost its history (blame).

Did you not copy nsSelection.cpp, rename and remove unnecessary lines from new file?
Assignee

Comment 8

2 years ago
I did a "hg copy" and then removed the lines, precisely to preserve history.
Well, I tried anyway - didn't it work correctly?

You can see this in part 2:
copy from dom/base/Selection.cpp
copy to layout/generic/nsFrameSelection.cpp

and in part 3:
copy from dom/base/Selection.cpp
copy to dom/base/SelectionChangeListener.cpp
Oh, now, nsFrmaeSelection.cpp is displayed with the blame.
https://searchfox.org/mozilla-central/source/layout/generic/nsFrameSetFrame.cpp

I guess that it's a bug of searchfox.org or just delayed to take the history in its backend.
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #9)
> Oh, now, nsFrmaeSelection.cpp is displayed with the blame.
> https://searchfox.org/mozilla-central/source/layout/generic/nsFrameSetFrame.
> cpp
> 
> I guess that it's a bug of searchfox.org or just delayed to take the history
> in its backend.

Oops, I saw different file, sorry for the spam.
https://searchfox.org/mozilla-central/source/layout/generic/nsFrameSelection.cpp

So, looks like it's a bug of searchfox.org.
I emailed billm about the bug of searchfox.
You need to log in before you can comment on or make changes to this bug.