[Webkit-unassigned] [Bug 220925] [webkit-patch] Allow user to opt out of browser for viewing the diff

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=220925

--- Comment #10 from Angelos Oikonomopoulos <angelos at igalia.com> ---
(In reply to Alexey Proskuryakov from comment #8)
> > I'm not sure what 'this' refers to here.
> 
> Running in a session where the browser cannot display diffs. I imagine that
> it's pretty limited set of configurations that you'd need to detect.

I have no idea; also, I wonder if with some text-mode browsers the diff might be something a person can understand, but still hard to make out, in which case we'd need to make a judgement call.

> I guess another consideration is that the function that you are patching is
> more generic than just diff viewing. Wouldn't this also prevent viewing test
> results, and do we want that?

AFAICS, this patch will only affect diff viewing. can_open_url is used in exactly two places. The first when deciding how to show the diff, the other is in open_url. open_url is used in various places, but it only uses the result of can_open_url to print a warning and then proceeds to unconditionally try and open the url in a browser:

    def open_url(self, url):
        if not self.can_open_url():
            _log.warn("Failed to open %s" % url)
        webbrowser.open(url)

So I think this should be pretty safe ATM. Though perhaps I should rename the variable to WEBKIT_PATCH_PREFER_PAGER and only check it in ConfirmDiff._show_pretty_diff.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210126/046119e4/attachment-0001.htm>