patch 8.1.1452: line and col property of popup windows not properly checked

Problem:    Line and col property of popup windows not properly checked.
Solution:   Check for "+" or "-" sign.

src/dict.c

@@ -604,6 +604,27 @@ dict_get_number(dict_T *d, char_u *key)
     return tv_get_number(&di->di_tv);
 }
 
+/*
+ * Get a number item from a dictionary.
+ * Returns 0 if the entry doesn't exist.
+ * Give an error if the entry is not a number.
+ */
+    varnumber_T
+dict_get_number_check(dict_T *d, char_u *key)
+{
+    dictitem_T	*di;
+
+    di = dict_find(d, key, -1);
+    if (di == NULL)
+	return 0;
+    if (di->di_tv.v_type != VAR_NUMBER)
+    {
+	semsg(_(e_invarg2), tv_get_string(&di->di_tv));
+	return 0;
+    }
+    return tv_get_number(&di->di_tv);
+}
+
 /*
  * Return an allocated string with the string representation of a Dictionary.
  * May return NULL.

src/popupwin.c

@@ -29,7 +29,7 @@ static poppos_entry_T poppos_entries[] = {
 };
 
 /*
- * Get option value for"key", which is "line" or "col".
+ * Get option value for "key", which is "line" or "col".
  * Handles "cursor+N" and "cursor-N".
  */
     static int
@@ -47,13 +47,15 @@ popup_options_one(dict_T *dict, char_u *key)
 
     val = tv_get_string(&di->di_tv);
     if (STRNCMP(val, "cursor", 6) != 0)
-	return dict_get_number(dict, key);
+	return dict_get_number_check(dict, key);
 
     setcursor_mayforce(TRUE);
     s = val + 6;
     if (*s != NUL)
     {
-	n = strtol((char *)s, (char **)&endp, 10);
+	endp = s;
+	if (*skipwhite(s) == '+' || *skipwhite(s) == '-')
+	    n = strtol((char *)s, (char **)&endp, 10);
 	if (endp != NULL && *skipwhite(endp) != NUL)
 	{
 	    semsg(_(e_invexpr2), val);
@@ -902,7 +904,7 @@ f_popup_getpos(typval_T *argvars, typval_T *rettv)
 	dict_add_number(dict, "core_height", wp->w_height);
 
 	dict_add_number(dict, "visible",
-				       (wp->w_popup_flags & POPF_HIDDEN) == 0);
+		      win_valid(wp) && (wp->w_popup_flags & POPF_HIDDEN) == 0);
     }
 }
 

src/proto/dict.pro

@@ -25,6 +25,7 @@ long dict_len(dict_T *d);
 dictitem_T *dict_find(dict_T *d, char_u *key, int len);
 char_u *dict_get_string(dict_T *d, char_u *key, int save);
 varnumber_T dict_get_number(dict_T *d, char_u *key);
+varnumber_T dict_get_number_check(dict_T *d, char_u *key);
 char_u *dict2string(typval_T *tv, int copyID, int restore_copyID);
 int dict_get_tv(char_u **arg, typval_T *rettv, int evaluate);
 void dict_extend(dict_T *d1, dict_T *d2, char_u *action);

src/testdir/test_popupwin.vim

@@ -224,6 +224,92 @@ func Test_popup_all_corners()
   call delete('XtestPopupCorners')
 endfunc
 
+func Test_popup_in_tab()
+  " default popup is local to tab, not visible when in other tab
+  let winid = popup_create("text", {})
+  call assert_equal(1, popup_getpos(winid).visible)
+  tabnew
+  call assert_equal(0, popup_getpos(winid).visible)
+  quit
+  call assert_equal(1, popup_getpos(winid).visible)
+  popupclear
+
+  " global popup is visible in any tab
+  let winid = popup_create("text", {'tab': -1})
+  call assert_equal(1, popup_getpos(winid).visible)
+  tabnew
+  call assert_equal(1, popup_getpos(winid).visible)
+  quit
+  call assert_equal(1, popup_getpos(winid).visible)
+  popupclear
+endfunc
+
+func Test_popup_valid_arguments()
+  " Zero value is like the property wasn't there
+  let winid = popup_create("text", {"col": 0})
+  let pos = popup_getpos(winid)
+  call assert_inrange(&columns / 2 - 1, &columns / 2 + 1, pos.col)
+  popupclear
+
+  " using cursor column has minimum value of 1
+  let winid = popup_create("text", {"col": 'cursor-100'})
+  let pos = popup_getpos(winid)
+  call assert_equal(1, pos.col)
+  popupclear
+
+  " center
+  let winid = popup_create("text", {"pos": 'center'})
+  let pos = popup_getpos(winid)
+  let around = (&columns - pos.width) / 2
+  call assert_inrange(around - 1, around + 1, pos.col)
+  let around = (&lines - pos.height) / 2
+  call assert_inrange(around - 1, around + 1, pos.line)
+  popupclear
+endfunc
+
+func Test_popup_invalid_arguments()
+  call assert_fails('call popup_create(666, {})', 'E714:')
+  popupclear
+  call assert_fails('call popup_create("text", "none")', 'E715:')
+  popupclear
+
+  call assert_fails('call popup_create("text", {"col": "xxx"})', 'E475:')
+  popupclear
+  call assert_fails('call popup_create("text", {"col": "cursor8"})', 'E15:')
+  popupclear
+  call assert_fails('call popup_create("text", {"col": "cursor+x"})', 'E15:')
+  popupclear
+  call assert_fails('call popup_create("text", {"col": "cursor+8x"})', 'E15:')
+  popupclear
+
+  call assert_fails('call popup_create("text", {"line": "xxx"})', 'E475:')
+  popupclear
+  call assert_fails('call popup_create("text", {"line": "cursor8"})', 'E15:')
+  popupclear
+  call assert_fails('call popup_create("text", {"line": "cursor+x"})', 'E15:')
+  popupclear
+  call assert_fails('call popup_create("text", {"line": "cursor+8x"})', 'E15:')
+  popupclear
+
+  call assert_fails('call popup_create("text", {"pos": "there"})', 'E475:')
+  popupclear
+  call assert_fails('call popup_create("text", {"padding": "none"})', 'E714:')
+  popupclear
+  call assert_fails('call popup_create("text", {"border": "none"})', 'E714:')
+  popupclear
+  call assert_fails('call popup_create("text", {"borderhighlight": "none"})', 'E714:')
+  popupclear
+  call assert_fails('call popup_create("text", {"borderchars": "none"})', 'E714:')
+  popupclear
+
+  call assert_fails('call popup_create([{"text": "text"}, 666], {})', 'E715:')
+  popupclear
+  call assert_fails('call popup_create([{"text": "text", "props": "none"}], {})', 'E714:')
+  popupclear
+  call assert_fails('call popup_create([{"text": "text", "props": ["none"]}], {})', 'E715:')
+  popupclear
+endfunc
+
 func Test_win_execute_closing_curwin()
   split
   let winid = popup_create('some text', {})
@@ -593,6 +679,15 @@ func Test_popup_atcursor()
   call assert_equal(4, pos.line)
   call popup_close(winid)
 
+  " cursor in first line, popup in line 2
+  call cursor(1, 1)
+  redraw
+  let winid = popup_atcursor(['vim', 'is', 'great'], {})
+  redraw
+  let pos = popup_getpos(winid)
+  call assert_equal(2, pos.line)
+  call popup_close(winid)
+
   bwipe!
 endfunc
 

src/version.c

@@ -767,6 +767,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1452,
 /**/
     1451,
 /**/

src/window.c

@@ -1368,6 +1368,9 @@ win_init_some(win_T *newp, win_T *oldp)
     win_copy_options(oldp, newp);
 }
 
+/*
+ * Return TRUE if "win" is a global popup or a popup in the current tab page.
+ */
     static int
 win_valid_popup(win_T *win UNUSED)
 {
@@ -1418,6 +1421,11 @@ win_valid_any_tab(win_T *win)
 	    if (wp == win)
 		return TRUE;
 	}
+#ifdef FEAT_TEXT_PROP
+	for (wp = tp->tp_first_popupwin; wp != NULL; wp = wp->w_next)
+	    if (wp == win)
+		return TRUE;
+#endif
     }
     return win_valid_popup(win);
 }