Bug 196119 – REGRESSION (r243094): ePub files do not render or open in Books

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

ePub files do not render or open in Books since r243094.

[Chris Dumez]

<rdar://problem/49121686>

[Chris Dumez]

Created attachment 365644 [details]
WIP Patch

[EWS Watchlist]

Attachment 365644 [details] did not pass style-queue:


ERROR: Source/WebKit/UIProcess/WebProcessPool.cpp:1221:  Missing space before {  [whitespace/braces] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 365661 [details]
Patch

[EWS Watchlist]

Comment on attachment 365661 [details]
Patch

Attachment 365661 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11608723

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html

[EWS Watchlist]

Created attachment 365674 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Brady Eidson]

Comment on attachment 365661 [details]
Patch

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

Any testing possible?

> Source/WebKit/ChangeLog:9
> +        Do not delay the WebProcess launch until a load when contructing a Web view and the client

"Web view" is bizarre.

WebView?

> Source/WebKit/UIProcess/API/Cocoa/WKProcessGroup.mm:219
> +    // If the client can observer when the connection to the WebProcess injected bundle is established, then we cannot

can observe

[Chris Dumez]

(In reply to Brady Eidson from comment #7)
> Comment on attachment 365661 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365661&action=review
> 
> Any testing possible?
> 
> > Source/WebKit/ChangeLog:9
> > +        Do not delay the WebProcess launch until a load when contructing a Web view and the client
> 
> "Web view" is bizarre.
> 
> WebView?

WebView is the name of the class. It is a Web View, or maybe a web view.
https://developer.apple.com/design/human-interface-guidelines/ios/views/web-views/

How is it weird?

> 
> > Source/WebKit/UIProcess/API/Cocoa/WKProcessGroup.mm:219
> > +    // If the client can observer when the connection to the WebProcess injected bundle is established, then we cannot
> 
> can observe

OK.

[Chris Dumez]

Created attachment 365731 [details]
Patch

[Chris Dumez]

Comment on attachment 365731 [details]
Patch

Clearing flags on attachment: 365731

Committed r243379: <https://trac.webkit.org/changeset/243379>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

(In reply to Chris Dumez from comment #8)
> (In reply to Brady Eidson from comment #7)
> > Comment on attachment 365661 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=365661&action=review
> > 
> > Any testing possible?
> > 
> > > Source/WebKit/ChangeLog:9
> > > +        Do not delay the WebProcess launch until a load when contructing a Web view and the client
> > 
> > "Web view" is bizarre.
> > 
> > WebView?
> 
> WebView is the name of the class. It is a Web View, or maybe a web view.
> https://developer.apple.com/design/human-interface-guidelines/ios/views/web-
> views/
> 
> How is it weird?
> 

Agree with Brady: very bizarre. Out of curiosity, when someone tells you something is bizarre and you don’t see a reply back from them then why do you ignore the bizarreness without full justification (from a English grammar book in this case) and still commit? Is it because this is a minor thing in your eyes?  Just curious, because if that is considered acceptable then maybe I should adopt such manners as I always thought it was rude to do what you did and commit. Always felt compelled to go the extra mile to find the literature to prove correctness. By the way, none of what I have written is meant to offend or be taking as sarcasim. (Typos and bad grammar in my response due to typing on iPhone). I genuinely want to know if maybe I have an old world mentality on manners :D, cuz i’m old :D.

This is going to seem obvious on reflection, and i’m Not a grammar expert, but just looked at the linked document you provided and thinking when you are referring to a view that displays web content you don’t treat web as a proper noun and if you read the first paragraph on the linked page, first sentence, you will see our dev pub does just that “web view”. Not “Web view”. As Brady said, if your referring to the class then you write “WebView”. You only write “Web view” if the sentence starts with “web” and that falls out of English writing semantics revolving capitalization.

> > 
> > > Source/WebKit/UIProcess/API/Cocoa/WKProcessGroup.mm:219
> > > +    // If the client can observer when the connection to the WebProcess injected bundle is established, then we cannot
> > 
> > can observe
> 
> OK.

[Chris Dumez]

(In reply to Daniel Bates from comment #12)
> (In reply to Chris Dumez from comment #8)
> > (In reply to Brady Eidson from comment #7)
> > > Comment on attachment 365661 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=365661&action=review
> > > 
> > > Any testing possible?
> > > 
> > > > Source/WebKit/ChangeLog:9
> > > > +        Do not delay the WebProcess launch until a load when contructing a Web view and the client
> > > 
> > > "Web view" is bizarre.
> > > 
> > > WebView?
> > 
> > WebView is the name of the class. It is a Web View, or maybe a web view.
> > https://developer.apple.com/design/human-interface-guidelines/ios/views/web-
> > views/
> > 
> > How is it weird?
> > 
> 
> Agree with Brady: very bizarre. Out of curiosity, when someone tells you
> something is bizarre and you don’t see a reply back from them then why do
> you ignore the bizarreness without full justification (from a English
> grammar book in this case) and still commit? Is it because this is a minor
> thing in your eyes?  Just curious, because if that is considered acceptable
> then maybe I should adopt such manners as I always thought it was rude to do
> what you did and commit. Always felt compelled to go the extra mile to find
> the literature to prove correctness. By the way, none of what I have written
> is meant to offend or be taking as sarcasim. (Typos and bad grammar in my
> response due to typing on iPhone). I genuinely want to know if maybe I have
> an old world mentality on manners :D, cuz i’m old :D.
> 
> This is going to seem obvious on reflection, and i’m Not a grammar expert,
> but just looked at the linked document you provided and thinking when you
> are referring to a view that displays web content you don’t treat web as a
> proper noun and if you read the first paragraph on the linked page, first
> sentence, you will see our dev pub does just that “web view”. Not “Web
> view”. As Brady said, if your referring to the class then you write
> “WebView”. You only write “Web view” if the sentence starts with “web” and
> that falls out of English writing semantics revolving capitalization.

I used "web view" in my patch, with lower case w.

[Daniel Bates]

(In reply to Chris Dumez from comment #13)
> (In reply to Daniel Bates from comment #12)
> > (In reply to Chris Dumez from comment #8)
> > > (In reply to Brady Eidson from comment #7)
> > > > Comment on attachment 365661 [details]
> > > > Patch
> > > > 
> > > > View in context:
> > > > https://bugs.webkit.org/attachment.cgi?id=365661&action=review
> > > > 
> > > > Any testing possible?
> > > > 
> > > > > Source/WebKit/ChangeLog:9
> > > > > +        Do not delay the WebProcess launch until a load when contructing a Web view and the client
> > > > 
> > > > "Web view" is bizarre.
> > > > 
> > > > WebView?
> > > 
> > > WebView is the name of the class. It is a Web View, or maybe a web view.
> > > https://developer.apple.com/design/human-interface-guidelines/ios/views/web-
> > > views/
> > > 
> > > How is it weird?
> > > 
> > 
> > Agree with Brady: very bizarre. Out of curiosity, when someone tells you
> > something is bizarre and you don’t see a reply back from them then why do
> > you ignore the bizarreness without full justification (from a English
> > grammar book in this case) and still commit? Is it because this is a minor
> > thing in your eyes?  Just curious, because if that is considered acceptable
> > then maybe I should adopt such manners as I always thought it was rude to do
> > what you did and commit. Always felt compelled to go the extra mile to find
> > the literature to prove correctness. By the way, none of what I have written
> > is meant to offend or be taking as sarcasim. (Typos and bad grammar in my
> > response due to typing on iPhone). I genuinely want to know if maybe I have
> > an old world mentality on manners :D, cuz i’m old :D.
> > 
> > This is going to seem obvious on reflection, and i’m Not a grammar expert,
> > but just looked at the linked document you provided and thinking when you
> > are referring to a view that displays web content you don’t treat web as a
> > proper noun and if you read the first paragraph on the linked page, first
> > sentence, you will see our dev pub does just that “web view”. Not “Web
> > view”. As Brady said, if your referring to the class then you write
> > “WebView”. You only write “Web view” if the sentence starts with “web” and
> > that falls out of English writing semantics revolving capitalization.
> 
> I used "web view" in my patch, with lower case w.

Nope, you didn’t. Try again:

<https://trac.webkit.org/changeset/243379/webkit#file1>

[Chris Dumez]

(In reply to Daniel Bates from comment #14)
> (In reply to Chris Dumez from comment #13)
> > (In reply to Daniel Bates from comment #12)
> > > (In reply to Chris Dumez from comment #8)
> > > > (In reply to Brady Eidson from comment #7)
> > > > > Comment on attachment 365661 [details]
> > > > > Patch
> > > > > 
> > > > > View in context:
> > > > > https://bugs.webkit.org/attachment.cgi?id=365661&action=review
> > > > > 
> > > > > Any testing possible?
> > > > > 
> > > > > > Source/WebKit/ChangeLog:9
> > > > > > +        Do not delay the WebProcess launch until a load when contructing a Web view and the client
> > > > > 
> > > > > "Web view" is bizarre.
> > > > > 
> > > > > WebView?
> > > > 
> > > > WebView is the name of the class. It is a Web View, or maybe a web view.
> > > > https://developer.apple.com/design/human-interface-guidelines/ios/views/web-
> > > > views/
> > > > 
> > > > How is it weird?
> > > > 
> > > 
> > > Agree with Brady: very bizarre. Out of curiosity, when someone tells you
> > > something is bizarre and you don’t see a reply back from them then why do
> > > you ignore the bizarreness without full justification (from a English
> > > grammar book in this case) and still commit? Is it because this is a minor
> > > thing in your eyes?  Just curious, because if that is considered acceptable
> > > then maybe I should adopt such manners as I always thought it was rude to do
> > > what you did and commit. Always felt compelled to go the extra mile to find
> > > the literature to prove correctness. By the way, none of what I have written
> > > is meant to offend or be taking as sarcasim. (Typos and bad grammar in my
> > > response due to typing on iPhone). I genuinely want to know if maybe I have
> > > an old world mentality on manners :D, cuz i’m old :D.
> > > 
> > > This is going to seem obvious on reflection, and i’m Not a grammar expert,
> > > but just looked at the linked document you provided and thinking when you
> > > are referring to a view that displays web content you don’t treat web as a
> > > proper noun and if you read the first paragraph on the linked page, first
> > > sentence, you will see our dev pub does just that “web view”. Not “Web
> > > view”. As Brady said, if your referring to the class then you write
> > > “WebView”. You only write “Web view” if the sentence starts with “web” and
> > > that falls out of English writing semantics revolving capitalization.
> > 
> > I used "web view" in my patch, with lower case w.
> 
> Nope, you didn’t. Try again:
> 
> <https://trac.webkit.org/changeset/243379/webkit#file1>

I guess I missed an instance, I am heartbroken.