From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?B?7KGw7ISx67mI?= Newsgroups: gmane.emacs.devel Subject: Re: [PATCH v5] Enable xwidgets on macOS Date: Wed, 24 Jul 2019 03:36:45 +0900 Message-ID: References: <20190718192321.65684-1-pcr910303@icloud.com> <20190719041654.88561-1-pcr910303@icloud.com> <83pnm5t8ui.fsf@gnu.org> Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="52332"; mail-complaints-to="usenet@blaine.gmane.org" Cc: alan@idiocy.org, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Jul 23 20:37:02 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hpzen-000DPu-FI for ged-emacs-devel@m.gmane.org; Tue, 23 Jul 2019 20:37:02 +0200 Original-Received: from localhost ([::1]:46556 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpzem-0006LS-H3 for ged-emacs-devel@m.gmane.org; Tue, 23 Jul 2019 14:37:00 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:42274) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hpzeh-0006L4-UG for emacs-devel@gnu.org; Tue, 23 Jul 2019 14:36:58 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hpzee-0004y2-IB for emacs-devel@gnu.org; Tue, 23 Jul 2019 14:36:55 -0400 Original-Received: from pv50p00im-ztdg10012001.me.com ([17.58.6.51]:60144) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hpzee-0004xB-7P for emacs-devel@gnu.org; Tue, 23 Jul 2019 14:36:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1563907009; bh=jg66STDnlkgMyQF2on/jA8h6umTuv76QmqF8s5TXIjQ=; h=Content-Type:Subject:From:Date:Message-Id:To; b=HnWIs5HjBkxqfcaskNkUrOxZr2WVL7pZwQDWItoC8U4YtJZLHD4Biry/Ie8YYdeTT Hpa+JGuUIetFKKR3PUFGxUXsGc8ZdCFG0gUnWXdS6TS93Yj9cGX+txcLgBnv2UxeC4 yOUYNFL6gdmR7H+GCVatxU6jd45f0o40hfe8TsSq3xzNXzYbNtd5+aBPUJxUBWiZWM xS0wNCQzbTzk3imp8FXJj/tQjQCFIGjCE9OJIFmPvCMWC84qnr1ky+ACTZUndv4bcv FthFpJMUrPLNWY7RTBK1wKzx8Bi78Ojlc0Jv3UZVnLx2pHv8mc7mjIbOIZ06asFzJU WhHDaseYelkWQ== Original-Received: from [192.168.0.11] (unknown [1.230.108.64]) by pv50p00im-ztdg10012001.me.com (Postfix) with ESMTPSA id 782082804E2; Tue, 23 Jul 2019 18:36:48 +0000 (UTC) In-Reply-To: <83pnm5t8ui.fsf@gnu.org> X-Mailer: Apple Mail (2.3445.9.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2019-07-23_08:, , signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=9 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1812120000 definitions=main-1907230187 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 17.58.6.51 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:238856 Archived-At: > 2019. 7. 20. =EC=98=A4=ED=9B=84 5:21, Eli Zaretskii = =EC=9E=91=EC=84=B1: >=20 >> From: Sungbin Jo >> Date: Fri, 19 Jul 2019 13:16:54 +0900 >> Cc: schwab@suse.de, alan@idiocy.org, eggert@cs.ucla.edu, >> monnier@iro.umontreal.ca, Sungbin Jo >>=20 >> --- >> configure.ac | 34 +- >> lisp/xwidget.el | 326 +++++++++++++---- >> nextstep/templates/Info.plist.in | 12 +- >> src/Makefile.in | 1 + >> src/emacs.c | 2 +- >> src/nsterm.m | 22 +- >> src/nsxwidget.h | 80 ++++ >> src/nsxwidget.m | 611 = +++++++++++++++++++++++++++++++ >> src/xwidget.c | 255 ++++++++++++- >> src/xwidget.h | 50 ++- >> 10 files changed, 1289 insertions(+), 104 deletions(-) >> create mode 100644 src/nsxwidget.h >> create mode 100644 src/nsxwidget.m >=20 > Thanks for working on this. >=20 > Please include the commit log message, formatted according to > ChangeLog rules (see CONTRIBUTE for the details). Some of the > questions below are because the log message was missing, and the > rationale for some changes was therefore not stated. Ok, I=E2=80=99ll include the log message when committing. > Also, I see that this changeset includes changes that are unrelated to > macOS support of xwidgets; I suggest to separate these two parts, as > the other part should be considered in its effect on all platforms. AFAIU, I should split these changes into two-or-more commits. Am I = correct? >> +(defun xwidget-webkit-split-below () >> + "Clone current URL into a new widget place in new window below. >> +Get the URL of current session, then browse to the URL >> +in `split-window-below' with a new xwidget webkit session." >=20 > The second sentence is unclear: what do you mean by "browse to the URL > in `split-window-below'"? How does one "browse to" something, and > what does "in" mean when `split-window-below' is a command? These functions used to be binded on split-window-{below,right}. It used to create a new xwidget webkit session as the usual = split-window-{below,right} behavior is to display the same buffer, but xwidget webkit in macOS = Cocoa doesn=E2=80=99t support simultaneously displaying it in the same time. These functions=E2=80=99 names changes to = `xwidget-webkit-clone-and-split-below' in the next patch. >> +(defun xwidget-webkit-split-right () >> + "Clone current URL into a new widget place in new window right. >> +Get the URL of current session, then browse to the URL >> +in `split-window-right' with a new xwidget webkit session." >=20 > Same here. >=20 >> (define-key map (kbd "SPC") = 'xwidget-webkit-scroll-up) >> + (define-key map (kbd "S-SPC") = 'xwidget-webkit-scroll-down) >=20 > Couldn't this new binding be a nuisance if the user holds the Shift > key for some reason? This binding (S-SPC) exists in at least in macOS Safari & FF. I think if one uses SPC to scroll up the view, they will expect S-SPC to = scroll down. >> - (define-key map [remap scroll-up] = 'xwidget-webkit-scroll-up) >> + (define-key map [remap scroll-up] = 'xwidget-webkit-scroll-up-line) >> (define-key map [remap scroll-up-command] = 'xwidget-webkit-scroll-up) >>=20 >> - (define-key map [remap scroll-down] = 'xwidget-webkit-scroll-down) >> + (define-key map [remap scroll-down] = 'xwidget-webkit-scroll-down-line) > [...] >> - (define-key map [remap previous-line] = 'xwidget-webkit-scroll-down) >> - (define-key map [remap next-line] = 'xwidget-webkit-scroll-up) >> + (define-key map [remap previous-line] = 'xwidget-webkit-scroll-down-line) >> + (define-key map [remap next-line] = 'xwidget-webkit-scroll-up-line) >=20 > What is the rationale for these changes? The functions are now configurable. The ones without the -line postfix gets an interactive argument of pixel = values. The ones with them gets an interactive argument of line numbers, and the = value of line height in pixel is a customizable variable. >> (define-key map [remap scroll-down-command] = 'xwidget-webkit-scroll-down) >>=20 >> (define-key map [remap forward-char] = 'xwidget-webkit-scroll-forward) >> (define-key map [remap backward-char] = 'xwidget-webkit-scroll-backward) >> (define-key map [remap right-char] = 'xwidget-webkit-scroll-forward) >> (define-key map [remap left-char] = 'xwidget-webkit-scroll-backward) >>=20 >> ;; (define-key map [remap move-beginning-of-line] 'image-bol) >> ;; (define-key map [remap move-end-of-line] 'image-eol) >> (define-key map [remap beginning-of-buffer] = 'xwidget-webkit-scroll-top) >> (define-key map [remap end-of-buffer] = 'xwidget-webkit-scroll-bottom) >> + ;; For macOS xwidget webkit, we don't support multiple views for = a >> + ;; model, instead, create a new session and model behind the = scene. >> + (when (memq window-system '(mac ns)) >> + (define-key map [remap split-window-below] = 'xwidget-webkit-split-below) >> + (define-key map [remap split-window-right] = 'xwidget-webkit-split-right)) >=20 > It could be confusing to have the same key invoke different commands > on different platforms. So maybe it is better to simply display a > message saying multiple views aren't supported on macOS? Reverted. >> +(defun xwidget-webkit-scroll-up (&optional n) >> + "Scroll webkit up by N pixels or window height pixels. >> +Stop if the bottom edge of the page is reached. >> +If N is omitted or nil, scroll up by window height pixels." >=20 > Suggest to rename N to ARG and reword the doc string: >=20 > "Scroll webkit up by ARG pixels; or full window height if no ARG. > Stop if bottom of page is reached. > Interactively, ARG is the prefix numeric argument. > Negative ARG scrolls down.=E2=80=9D Ok. >> +(defun xwidget-webkit-scroll-down (&optional n) >> + "Scroll webkit down by N pixels or window height pixels. >> +Stop if the top edge of the page is reached. >> +If N is omitted or nil, scroll down by window height pixels." >=20 > Same here. Ok. >> +(defcustom xwidget-webkit-scroll-line-height 50 >> + "Default line height in pixels to scroll xwidget webkit." >> + :type 'integer) > [...] >> +(defcustom xwidget-webkit-scroll-char-width 10 >> + "Default char height in pixels to scroll xwidget webkit." >> + :type 'integer) >=20 > Why do we need these defaults? Can't we use the height and the width > of the default face's font instead? Removed defcustom and changed related functions to use = `window-font-height=E2=80=99 and `window-font-width'. >> +(defun xwidget-webkit-scroll-forward (&optional n) >> + "Scroll webkit forwards by N chars. >=20 > I suggest to say "scroll horizontally, otherwise "by N characters" > sounds surprising. Ok. >> +(defun xwidget-webkit-scroll-backward (&optional n) >> + "Scroll webkit backwards by N chars. >=20 > Please use "back", not "backwards=E2=80=9D. Ok. >> @@ -184,7 +253,7 @@ xwidget-webkit-scroll-bottom >> (interactive) >> (xwidget-webkit-execute-script >> (xwidget-webkit-current-session) >> - "window.scrollTo(pageXOffset, = window.document.body.clientHeight);")) >> + "window.scrollTo(pageXOffset, = window.document.body.scrollHeight);")) >=20 > Why this change? It=E2=80=99s a fix to scroll to the bottom of the page. The previous one has edge cases, e.g. when there are horizontal scroll = bars. >> @@ -219,43 +287,141 @@ xwidget-webkit-callback >> "error: callback called for xwidget with dead buffer") >> (with-current-buffer (xwidget-buffer xwidget) >> (cond ((eq xwidget-event-type 'load-changed) >> - (xwidget-webkit-execute-script >> - xwidget "document.title" >> - (lambda (title) >> - (xwidget-log "webkit finished loading: '%s'" title) >> - ;;TODO - check the native/internal scroll >> - ;;(xwidget-adjust-size-to-content xwidget) >> - (xwidget-webkit-adjust-size-to-window xwidget) >> - (rename-buffer (format "*xwidget webkit: %s *" = title)))) >> - (pop-to-buffer (current-buffer))) >> + ;; We do not change selected window for the finish of = loading a page. >> + ;; And do not adjust webkit size to window here, the = selected window >> + ;; can be the mini-buffer window unwantedly. >> + (let ((title (xwidget-webkit-title xwidget))) >> + (xwidget-log "webkit finished loading: %s" title) >> + (rename-buffer (format "*xwidget webkit: %s *" title) = t))) >> ((eq xwidget-event-type 'decide-policy) >> (let ((strarg (nth 3 last-input-event))) >> (if (string-match ".*#\\(.*\\)" strarg) >> (xwidget-webkit-show-id-or-named-element >> xwidget >> (match-string 1 strarg))))) >> + ;; TODO: Response handling other than download. >> + ((eq xwidget-event-type 'download-callback) >> + (let ((url (nth 3 last-input-event)) >> + (mime-type (nth 4 last-input-event)) >> + (file-name (nth 5 last-input-event))) >> + (xwidget-webkit-save-as-file url mime-type = file-name))) >> ((eq xwidget-event-type 'javascript-callback) >> (let ((proc (nth 3 last-input-event)) >> (arg (nth 4 last-input-event))) >> - (funcall proc arg))) >> + ;; Some javascript return vector as result >> + (funcall proc (if (vectorp arg) (seq-into arg 'list) = arg)))) >> (t (xwidget-log "unhandled event:%s" = xwidget-event-type)))))) >=20 > Can you explain the rationale for these changes? Hmm=E2=80=A6 The first hunk looks like it=E2=80=99s using the new = function `xwidget-webkit-title'. The call to `xwidget-webkit-adjust-size-to-window' was removed as the = webkit can finish reloading at any time, and if the mini-buffer is focused at the = point, the call misplaces the xwidget buffer. The second hunk is for handling download-callback events in xwidget.=20 The event generator function is there, but it is only called in Cocoa = Webkit. It=E2=80=99s not implemented in the GTK one, but it wouldn=E2=80=99t = matter running. I reverted the third hunk and related changes. >> +(defvar isearch-search-fun-function) >> +(when (memq window-system '(mac ns)) >> + (defcustom xwidget-webkit-enable-plugins nil >> + "Enable plugins for xwidget webkit. >> +If non-nil, plugins are enabled. Otherwise, disabled." >> + :type 'boolean)) >=20 > I think all defcustoms here should be platform-independent. You can > say in the doc string that the value is only used on macOS. Should I change it to a defvar? Or should I remove this variable at all? >> (define-derived-mode xwidget-webkit-mode >> special-mode "xwidget-webkit" "Xwidget webkit view mode." >> (setq buffer-read-only t) >> + (setq cursor-type nil) >=20 > Why this change? >=20 >> + (setq-local isearch-search-fun-function >> + #'xwidget-webkit-search-fun-function) >> + (setq-local isearch-lazy-highlight nil) >=20 > And these? I vaguely remember both were because of implementing website isearch. I remember isearch working months ago=E2=80=A6 but it doesn=E2=80=99t = now. :-( I reverted the related changes. I=E2=80=99ll take a look at them other time. >> +(defcustom xwidget-webkit-download-dir "~/Downloads/" >> + "Directory where download file saved." >=20 > "Directory where download files are saved." >=20 >> + :type 'string) >=20 > Shouldn't this be 'file instead? Ok. > Also, whenever you add or modify a defcustom, please always supply a > :version tag. Ok. >> +(defun xwidget-webkit-save-as-file (url mime-type &optional = file-name) >> + "For XWIDGET webkit, save URL resource of MIME-TYPE as FILE-NAME." >=20 > The doc string doesn't say what file name will be used if FILE-NAME is > omitted or nil. While fixing the doc string, I found out FILE-NAME shouldn=E2=80=99t be = an optional argument :-( Fixed. >> + (let ((save-name (read-file-name >> + (format "Save '%s' file as: " mime-type) >=20 > This prompt will be confusing. It will, for example say >=20 > Save app/xdiff file as: >=20 > Maybe this prompt would be slightly better: >=20 > (format "Save URL `%s' of type `%s' in file/directory: " url = mime-type) Ok. >> + (if (file-directory-p save-name) >> + (setq save-name >> + (expand-file-name (file-name-nondirectory file-name) = save-name))) >=20 > This should be mentioned in the doc string. Ok. >> +(defcustom xwidget-webkit-bookmark-jump-new-session nil >> + "Control bookmark jump to use new session or not. >> +If non-nil, it will use a new session. Otherwise, it will use >> +`xwidget-webkit-last-session'. When you set this variable to >> +nil, consider further customization with >> +`xwidget-webkit-last-session-buffer'." >> + :type 'boolean) >=20 > The first sentence will be more informative if it said >=20 > If non-nil, use a new xwidget webkit session after bookmark jump. >=20 > Also, a :version tag is missing. Ok. >> (defun xwidget-webkit-bookmark-make-record () >> "Integrate Emacs bookmarks with the webkit xwidget." >=20 > This doc string doesn't describe what the function does. Ok. >> +;; Initialize last search text length variable when isearch starts >=20 > Please add a period at the end of this sentence. >=20 >> +(add-hook 'isearch-mode-hook >> + (lambda () >> + (setq xwidget-webkit-isearch-last-length 0))) >> + >> +;; This is minimal. Regex and incremental search will be nice > ^^ > Our convention is to have two spaces between sentences. >=20 > More generally, I wonder whether its a good idea to unconditionally > add to isearch-mode-hook just because this file was loaded. Removed all isearch-related code. >> +(defvar xwidget-webkit-search-js " >> +var xwSearchForward =3D %s; >> +var xwSearchRepeat =3D %s; >> +var xwSearchString =3D '%s'; >> +if (window.getSelection() && !window.getSelection().isCollapsed) { >> + if (xwSearchRepeat) { >> + if (xwSearchForward) >> + window.getSelection().collapseToEnd(); >> + else >> + window.getSelection().collapseToStart(); >> + } else { >> + if (xwSearchForward) >> + window.getSelection().collapseToStart(); >> + else { >> + var sel =3D window.getSelection(); >> + window.getSelection().collapse(sel.focusNode, sel.focusOffset = + 1); >> + } >> + } >> +} >> +window.find(xwSearchString, false, !xwSearchForward, true, false, = true); >> +") >=20 > This defvar should have a doc string. >=20 >> +;; Utility functions, wanted in `window.el' >=20 > What do you mean by "wanted=E2=80=9D here? Removed the mention of `window.el'. >> @@ -506,23 +691,27 @@ xwidget-webkit-goto-url >> (defun xwidget-webkit-back () >> "Go back in history." >> (interactive) >> - (xwidget-webkit-execute-script (xwidget-webkit-current-session) >> - "history.go(-1);")) >> + (xwidget-webkit-goto-history (xwidget-webkit-current-session) -1)) >=20 > What is the rationale for this change? It=E2=80=99s using the newly defined function in C = `xwidget-webkit-goto-history'. >> +(defun xwidget-webkit-forward () >> + "Go forward in history." >=20 > The doc string should somehow make clear that this function is about > xwidgets. A reference to the history variable would also be nice. Changed doc string, but I=E2=80=99m not sure about what the history = variable you=E2=80=99re referencing is. >> (defun xwidget-webkit-current-url () >> - "Get the webkit url and place it on the kill-ring." >> + "Get the current xwidget webkit URL." >> (interactive) >> - (xwidget-webkit-execute-script >> - (xwidget-webkit-current-session) >> - "document.URL" (lambda (rv) >> - (let ((url (kill-new (or rv "")))) >> - (message "url: %s" url))))) >> + (xwidget-webkit-uri (xwidget-webkit-current-session))) >> + >> +(defun xwidget-webkit-current-url-message-kill () >> + "Display the current xwidget webkit URL and place it on the = `kill-ring'." >> + (interactive) >> + (message "URL: %s" (kill-new (or (xwidget-webkit-current-url) = "")))) >=20 > You are changing the user-visible behavior here. Why is that a good > idea? >=20 > In any case, user-visible changes should be called out in NEWS. IMHO, the new name is more clearer. However, I reverted to the previous behavior. >> + /* If the last rect is too large (ex, xwidget webkit), update at >> + every move, or resizing by dragging modeline or vertical split = is >> + very hard to make its way. */ >> + if (dragging && (r->size.width > 32 || r->size.height > 32)) >=20 > Is it wise to have these values hard-coded? What do these values > represent? Sorry, I=E2=80=99m not sure about how these values are selected. Looks like it=E2=80=99s just heuristics, but I=E2=80=99m not sure. >> - /* Has movement gone beyond last rect we were tracking? */ >> - if (x < r->origin.x || x >=3D r->origin.x + r->size.width >> + /* Has movement gone beyond last rect we were tracking? */ >=20 > Why did you change the comment here? Reverted. >> @@ -611,24 +660,59 @@ when there are (struct glyph_string *s) >> initialization. */ >> struct xwidget *xww =3D s->xwidget; >> struct xwidget_view *xv =3D xwidget_view_lookup (xww, s->w); >> + int text_area_x, text_area_y, text_area_width, text_area_height; >> int clip_right; >> int clip_bottom; >> int clip_top; >> int clip_left; >>=20 >> int x =3D s->x; >> - int y =3D s->y + (s->height / 2) - (xww->height / 2); >> + int y =3D s->y; >=20 > Why this change? I=E2=80=99m not sure why xwidgets should be top-aligned, it looks like = it=E2=80=99s due to personal preference of the original patch author. Reverted. >> + /* Resize xwidget webkit if its container window size is changed = in >> + some ways, for example, a buffer became hidden in small split >> + window, then it can appear front in merged whole window. */ >> + if (EQ (xww->type, Qwebkit) >> + && (xww->width !=3D text_area_width || xww->height !=3D = text_area_height)) >> + { >> + Lisp_Object xwl; >> + XSETXWIDGET (xwl, xww); >> + Fxwidget_resize (xwl, >> + make_int (text_area_width), >> + make_int (text_area_height)); >> + } >=20 > And this one? This is due to the removement of the call = `xwidget-webkit-adjust-size-to-window' explained above. >> +DEFUN ("xwidget-webkit-uri", >> + Fxwidget_webkit_uri, Sxwidget_webkit_uri, >> + 1, 1, 0, >> + doc: /* Get the current URL of XWIDGET webkit. */) >> + (Lisp_Object xwidget) >> +{ >> + WEBKIT_FN_INIT (); >> +#ifdef USE_GTK >> + WebKitWebView *wkwv =3D WEBKIT_WEB_VIEW (xw->widget_osr); >> + return build_string (webkit_web_view_get_uri (wkwv)); >> +#elif defined NS_IMPL_COCOA >> + return nsxwidget_webkit_uri (xw); >> +#endif >> +} >> + >> +DEFUN ("xwidget-webkit-title", >> + Fxwidget_webkit_title, Sxwidget_webkit_title, >> + 1, 1, 0, >> + doc: /* Get the current title of XWIDGET webkit. */) >> + (Lisp_Object xwidget) >> +{ >> + WEBKIT_FN_INIT (); >> +#ifdef USE_GTK >> + WebKitWebView *wkwv =3D WEBKIT_WEB_VIEW (xw->widget_osr); >> + return build_string (webkit_web_view_get_title (wkwv)); >> +#elif defined NS_IMPL_COCOA >> + return nsxwidget_webkit_title (xw); >> +#endif >=20 > These new functions should be called out in NEWS. Ok. >> +DEFUN ("xwidget-webkit-goto-history", >> + Fxwidget_webkit_goto_history, Sxwidget_webkit_goto_history, >> + 2, 2, 0, >> + doc: /* Make the XWIDGET webkit load REL-POS (-1, 0, 1) page = in browse history. */) >> + (Lisp_Object xwidget, Lisp_Object rel_pos) >> +{ >> + WEBKIT_FN_INIT (); >> + CHECK_RANGED_INTEGER (rel_pos, -1, 1); /* -1, 0, 1 */ >> +#ifdef USE_GTK >> + WebKitWebView *wkwv =3D WEBKIT_WEB_VIEW (xw->widget_osr); >> + switch (XFIXNAT (rel_pos)) { >> + case -1: webkit_web_view_go_back (wkwv); break; >> + case 0: webkit_web_view_reload (wkwv); break; >> + case 1: webkit_web_view_go_forward (wkwv); break; >> + } >> +#elif defined NS_IMPL_COCOA >> + nsxwidget_webkit_goto_history (xw, XFIXNAT (rel_pos)); >> +#endif >> return Qnil; >> } >=20 > Likewise. >=20 >> @@ -759,11 +930,12 @@ DEFUN ("xwidget-webkit-execute-script", >> { >> WEBKIT_FN_INIT (); >> CHECK_STRING (script); >> - if (!NILP (fun) && !FUNCTIONP (fun)) >> + if (!FUNCTIONP (fun)) >> wrong_type_argument (Qinvalid_function, fun); >=20 > Why this change? That was an mistake. Reverted. >> +#elif defined NS_IMPL_COCOA >> + return nsxwidget_get_size(XXWIDGET (xwidget)); > ^^ > Our convention is to leave one space between the name of a function > and the opening parenthesis of the argument list. Fixed. >> @@ -909,14 +1098,19 @@ DEFUN ("delete-xwidget-view", >> { >> CHECK_XWIDGET_VIEW (xwidget_view); >> struct xwidget_view *xv =3D XXWIDGET_VIEW (xwidget_view); >> - gtk_widget_destroy (xv->widgetwindow); >> Vxwidget_view_list =3D Fdelq (xwidget_view, Vxwidget_view_list); >> +#ifdef USE_GTK >> + gtk_widget_destroy (xv->widgetwindow); >=20 > This changes the order of calls. What is the reason for that? Reverted. > Thanks again for working on this.