321433 - Don't enable selection for printing

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect)

Description

[Eli Friedman]

http://lxr.mozilla.org/seamonkey/source/layout/base/nsPresShell.cpp#1723 is wrong.

Comment 1

[Eli Friedman]

Attachment: Patch

Comment 2

[Eli Friedman]

Comment on attachment 206771 [details] [diff] [review]
Patch

Any thoughts on this?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Does printing only the selection still work?

Comment 4

[Eli Friedman]

Printing selection seems to work fine; I'm pretty sure selection controllers are per-presshell is per-PresShell.

Some thoughts that might help:
This acts as kind of an alternate, more comprehensive fix to bug 312415, I guess; it stops the selection painting/event code from being used in the first place.  Text selection never displayed anyway when printing because of the weirdness of nsTextFrame::GetTextInfoForPainting, but I don't think that's really the right way to stop image selection from showing.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

sorry, forgot to CC myself before.

With this, can we remove the special-case selection disabling from nsTextFrame?

Comment 6

[Eli Friedman]

I suppose, but is it worth bothering?  Isn't there some plan in the works to rewrite nsTextFrame, or at least the painting?  That would make deleting the extra code moot, wouldn't it?  Text painting is a serious mess at the moment, and it really needs a rewrite; very similar code doesn't really need to be repeated three times.  Actually, the removal would need to touch all three versions.  So, I'll do it if you like.

The fix to bug 312415 could also be removed.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 206771 [details] [diff] [review]
Patch

alright, let's go with this

Comment 8

[Eli Friedman]

Attachment: Patch with some cleanup

I think this is the version I'd like to check in; this includes a couple simple cleanups without any messy changes.

If this patch is OK with you, do I need someone else to superreview?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 209992 [details] [diff] [review]
Patch with some cleanup

this is fine.

Comment 10

[Eli Friedman]

Don't check in the last patch; I just realized how the text painting logic works.  I really hate badly labeled stuff.

I couldn't test the last version properly because print clipping is broken, but the nsTextFrame changes are wrong.

Comment 11

[Eli Friedman]

Attachment: Final version

Correct nsTextFrame.cpp changes.  No other differences.

Comment 12

[Eli Friedman]

Checked in.