Bug 148520

Summary: Text areas resizer and scrollbars shouldn't be rendered while printing
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: PrintingAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: commit-queue, darin, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, tonikitoo, zalan
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Updated patch darin: review+

Description Carlos Garcia Campos 2015-08-27 02:55:37 PDT
The resizer is useless in a printed document. Some HTML forms that are printed to be filled manually look really weird with the resizer in the text areas.
Comment 1 Carlos Garcia Campos 2015-08-27 02:58:24 PDT
Created attachment 260049 [details]
Patch
Comment 2 Simon Fraser (smfr) 2015-08-27 03:04:25 PDT
Comment on attachment 260049 [details]
Patch

Isn't the same true of scrollbars? Why is this different?
Comment 3 Carlos Garcia Campos 2015-08-27 03:10:08 PDT
(In reply to comment #2)
> Comment on attachment 260049 [details]
> Patch
> 
> Isn't the same true of scrollbars? Why is this different?

Yes, I guess it also applies to scrollbars, I noticed it after printing a form with text areas, but no scrollbars.
Comment 4 Carlos Garcia Campos 2015-11-20 02:42:13 PST
Created attachment 265946 [details]
Updated patch

Also hide the scrollbars and include a layout test.
Comment 5 Darin Adler 2015-11-30 13:17:53 PST
Comment on attachment 265946 [details]
Updated patch

Patch is OK but I’d prefer not to make the printing mechanism, already messy, so much messier. I think this makes things a little worse by sprinkling more printing-specific code in some, but not all, places that deal with scroll bars. Originally I had many comments about how to simplify this, but now I think I have only one comment: Seems we’d want to use the style system for this if possible instead of adding special case code for printing. There’s a media type of "print" and we can eliminate both scrollbars and resizers with !important style rules. Can we try this?

@media print {
    ::-webkit-scrollbar { display: none !important; }
    ::-webkit-resizer { display: none !important; }
}

Perhaps this can go right into html.css? If not, maybe there’s some other way to introduce those style rules at the right time.

I suspect this will eliminate the need for any code changes in RenderLayer and RenderListBox; some small changes to FrameView or ScrollView might be needed, but they would likely be minimal, and also likely would fix bugs where these style rules aren’t properly respected.

Another thought: I don’t think the current patch covers all the cases. What about RenderMenuList and RenderSearchField? Should cover those with test cases too.

I’m going to say r=me but I’d like to see a version with fewer code changes. The printing code should be reduced in complexity if possible rather than increased.
Comment 6 Carlos Garcia Campos 2015-12-01 00:45:23 PST
(In reply to comment #5)
> Comment on attachment 265946 [details]
> Updated patch
> 
> Patch is OK but I’d prefer not to make the printing mechanism, already
> messy, so much messier. I think this makes things a little worse by
> sprinkling more printing-specific code in some, but not all, places that
> deal with scroll bars. Originally I had many comments about how to simplify
> this, but now I think I have only one comment: Seems we’d want to use the
> style system for this if possible instead of adding special case code for
> printing. There’s a media type of "print" and we can eliminate both
> scrollbars and resizers with !important style rules. Can we try this?
> 
> @media print {
>     ::-webkit-scrollbar { display: none !important; }
>     ::-webkit-resizer { display: none !important; }
> }
> 
> Perhaps this can go right into html.css? If not, maybe there’s some other
> way to introduce those style rules at the right time.

Great, idea, I'll try that. I also thought about something similar, injecting a user style sheet before printing and removing it afterwards, but this looks a lot cleaner to me.

> I suspect this will eliminate the need for any code changes in RenderLayer
> and RenderListBox; some small changes to FrameView or ScrollView might be
> needed, but they would likely be minimal, and also likely would fix bugs
> where these style rules aren’t properly respected.
> 
> Another thought: I don’t think the current patch covers all the cases. What
> about RenderMenuList and RenderSearchField? Should cover those with test
> cases too.

In both cases the scrollbars are in the popups, no? so I assumed the popups are always hidden when printing.

> I’m going to say r=me but I’d like to see a version with fewer code changes.
> The printing code should be reduced in complexity if possible rather than
> increased.

I agree, thanks for the review.