[webkit-reviews] review granted: [Bug 178267] Clicks on Link with download attribute causes all (other) links to trigger download when clicked : [Attachment 323844] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 178267: Clicks on Link with download attribute causes all (other) links to
trigger download when clicked
https://bugs.webkit.org/show_bug.cgi?id=178267

Attachment 323844: Patch

https://bugs.webkit.org/attachment.cgi?id=323844&action=review




--- Comment #17 from Darin Adler <darin at apple.com> ---
Comment on attachment 323844
  --> https://bugs.webkit.org/attachment.cgi?id=323844
Patch

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

>>> Source/WebKit/UIProcess/API/APINavigation.h:73
>>> +	 bool m_shouldForceDownload { false };
>> 
>> I'm not sure the should is getting you much here.  Maybe, just
forceDownload?
> 
> I thought this was part of our coding style for Booleans. I personally like
it better with the should prefix too.

The rule is inspired by Cocoa; it’s to help make sure that getters don’t sound
like verb phrases. A function called “force download” doesn’t necessarily sound
like a getter to everyone.

I like the way the "should" decreases ambiguity in the function name. And I
also like booleans that work as a sentence "navigation should force download"
as opposed to "navigation force download".

> LayoutTests/imported/w3c/ChangeLog:13
> +	   *
web-platform-tests/html/browsers/windows/noreferrer-window-name-expected.txt:

Do these tests that rely things that have to time out slow down test runs
overall, or is there some reason they don’t?