Bug 196478 – [WK2] Add support for Window's beforeprint / afterprint events

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Add support for Window's beforeprint / afterprint events as per:
- https://html.spec.whatwg.org/#dom-print

Blink and Gecko already support this.

[Chris Dumez]

Created attachment 366463 [details]
Patch

[Alex Christensen]

Comment on attachment 366463 [details]
Patch

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

> Tools/WebKitTestRunner/TestController.cpp:268
> +    WKPageBeginPrinting(page, frame, WKPrintInfo { 1.0, 21.0, 29.7 });
> +    WKPageEndPrinting(page);

The definition of these is inside #if PLATFORM(COCOA).  I don't see any reason why.

[Chris Dumez]

(In reply to Alex Christensen from comment #2)
> Comment on attachment 366463 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366463&action=review
> 
> > Tools/WebKitTestRunner/TestController.cpp:268
> > +    WKPageBeginPrinting(page, frame, WKPrintInfo { 1.0, 21.0, 29.7 });
> > +    WKPageEndPrinting(page);
> 
> The definition of these is inside #if PLATFORM(COCOA).  I don't see any
> reason why.

Indeed, this was not intentional.

[Chris Dumez]

Created attachment 366515 [details]
Patch

[Chris Dumez]

Created attachment 366526 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 366526 [details]
Patch

Clearing flags on attachment: 366526

Committed r243762: <https://trac.webkit.org/changeset/243762>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49535124>

[Fujii Hironori]

Committed r243783: <https://trac.webkit.org/changeset/243783>

[Shawn Roberts]

It looks like changes in https://trac.webkit.org/changeset/243762/webkit is causing the following layout test to crash in iOS Simulator Debug

editing/execCommand/print.html

Is crashing 100% of the time locally and on the bots. Does not happen in prior revisions.

Reproduced with:

run-webkit-tests editing/execCommand/print.html --iterations 5 -f --debug --ios-simulator

Crash Log:

https://build.webkit.org/results/Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r243933%20(3157)/editing/execCommand/print-crash-log.txt

Same crash locally.

[Chris Dumez]

(In reply to Shawn Roberts from comment #10)
> It looks like changes in https://trac.webkit.org/changeset/243762/webkit is
> causing the following layout test to crash in iOS Simulator Debug
> 
> editing/execCommand/print.html
> 
> Is crashing 100% of the time locally and on the bots. Does not happen in
> prior revisions.
> 
> Reproduced with:
> 
> run-webkit-tests editing/execCommand/print.html --iterations 5 -f --debug
> --ios-simulator
> 
> Crash Log:
> 
> https://build.webkit.org/results/
> Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r243933%20(3157)/
> editing/execCommand/print-crash-log.txt
> 
> Same crash locally.

Probably a side effect of calling beginPrinting() in WebKitTestRunner. I guess it uncovered a bug.

[Chris Dumez]

(In reply to Chris Dumez from comment #11)
> (In reply to Shawn Roberts from comment #10)
> > It looks like changes in https://trac.webkit.org/changeset/243762/webkit is
> > causing the following layout test to crash in iOS Simulator Debug
> > 
> > editing/execCommand/print.html
> > 
> > Is crashing 100% of the time locally and on the bots. Does not happen in
> > prior revisions.
> > 
> > Reproduced with:
> > 
> > run-webkit-tests editing/execCommand/print.html --iterations 5 -f --debug
> > --ios-simulator
> > 
> > Crash Log:
> > 
> > https://build.webkit.org/results/
> > Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r243933%20(3157)/
> > editing/execCommand/print-crash-log.txt
> > 
> > Same crash locally.
> 
> Probably a side effect of calling beginPrinting() in WebKitTestRunner. I
> guess it uncovered a bug.

Ryosuke is more familiar with this code:
#ifndef NDEBUG
    // we should always be able to make the affinity DOWNSTREAM, because going previous from an
    // UPSTREAM position can never yield another UPSTREAM position (unless line wrap length is 0!).
    if (prev.isNotNull() && m_affinity == UPSTREAM) {
        VisiblePosition temp = prev;
        temp.setAffinity(UPSTREAM);
        ASSERT(inSameLine(temp, prev));
    }
#endif

We hit this assertion in VisiblePosition::previous().

[Ryosuke Niwa]

(In reply to Chris Dumez from comment #12)
> (In reply to Chris Dumez from comment #11)
> > (In reply to Shawn Roberts from comment #10)
> > > It looks like changes in https://trac.webkit.org/changeset/243762/webkit is
> > > causing the following layout test to crash in iOS Simulator Debug
> > > 
> > > editing/execCommand/print.html
> > > 
> > > Is crashing 100% of the time locally and on the bots. Does not happen in
> > > prior revisions.
> > > 
> > > Reproduced with:
> > > 
> > > run-webkit-tests editing/execCommand/print.html --iterations 5 -f --debug
> > > --ios-simulator
> > > 
> > > Crash Log:
> > > 
> > > https://build.webkit.org/results/
> > > Apple%20iOS%2012%20Simulator%20Debug%20WK2%20(Tests)/r243933%20(3157)/
> > > editing/execCommand/print-crash-log.txt
> > > 
> > > Same crash locally.
> > 
> > Probably a side effect of calling beginPrinting() in WebKitTestRunner. I
> > guess it uncovered a bug.
> 
> Ryosuke is more familiar with this code:
> #ifndef NDEBUG
>     // we should always be able to make the affinity DOWNSTREAM, because
> going previous from an
>     // UPSTREAM position can never yield another UPSTREAM position (unless
> line wrap length is 0!).
>     if (prev.isNotNull() && m_affinity == UPSTREAM) {
>         VisiblePosition temp = prev;
>         temp.setAffinity(UPSTREAM);
>         ASSERT(inSameLine(temp, prev));
>     }
> #endif
> 
> We hit this assertion in VisiblePosition::previous().

We're not gonna be able to fix such an assertion anytime soon. I suggest we skip the test in debug builds for now.

[Chris Dumez]

*** Bug 19937 has been marked as a duplicate of this bug. ***