GNU bug report logs - #29565
[PATCH] Support xwidget webkit for macOS X

Previous Next

Full log

Message #8 received at 29565 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Jaesup Kwak <veshboo <at> gmail.com>
Cc: 29565 <at> debbugs.gnu.org
Subject: Re: bug#29565: [PATCH] Support xwidget webkit for macOS X
Date: Mon, 4 Dec 2017 20:59:43 +0000

On Tue, Dec 05, 2017 at 01:44:42AM +0900, Jaesup Kwak wrote:
> I attached a patch to support xwidget webkit for macOS X, rebased onto the
> latest master.

Hi, thanks for working on this, it looks really good. I’ve had a quick
glance over it and I have a few comments.

In configure.ac you’re doing

    elif test "$window_system" = "nextstep"; then

I think instead of that you should be doing

    elif test "${NS_IMPL_COCOA}" = "yes"; then

as nextstep covers GNUstep, but it doesn’t have a webkit
implementation (yet).

There might be other places where NS_IMPL_COCOA is more appropriate
than HAVE_NS, but it builds fine against GNUstep with xwidgets off, so
it’s not too important.

I notice you’re adding .DS_Store to .gitignore, which is a good idea,
but I don’t think it should be done in this patch. It seems a little
off‐topic.

I’m also a little unsure about this

    <!-- FIXME: Uncomment for xwidget webkit to browse remote url,
         but this set no restriction at all.  Consult apple's documentation
         for detail information about `NSApplicationDefinedMask'. -->
    <key>NSAppTransportSecurity</key>
    <dict>
      <key>NSAllowsArbitraryLoads</key>
      <true/>
    </dict>

The comment says ‘uncomment’, but it *is* uncommented. Am I
misunderstanding?

(Also I think there should be two spaces at the end of comments.)

I think we’ll want to add NSTRACE lines in each function, but perhaps
we should create a new category for XWidgets.
-- 
Alan Third

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.