[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [Git][tpo/applications/tor-browser][tor-browser-128.2.0esr-14.0-1] Bug 1436462 - Use "Open in new private window" for bookmarks when in PBM. r=places-reviewers, mak

Title: GitLab

morgan pushed to branch tor-browser-128.2.0esr-14.0-1 at The Tor Project / Applications / Tor Browser

Commits:

  • 1d54c76d
    by Henry Wilkes at 2024-09-16T21:45:38+00:00 
    Bug 1436462 - Use "Open in new private window" for bookmarks when in PBM. r=places-reviewers,mak

This makes the bookmark menu consistent with the "File" and context
menu when using private browsing mode.

We also share the same hide item logic for these "open" items in one
place in PlacesUIUtils so that they can be shared between the two
consumers (regular bookmarks and managed bookmarks). This ensures that
the "Open in container" item if hidden for managed bookmarks in a
private window.

Differential Revision: https://phabricator.services.mozilla.com/D220120

5 changed files:

Changes:

  • browser/components/places/PlacesUIUtils.sys.mjs
    ... ... @@ -1514,6 +1514,40 @@ export var PlacesUIUtils = {
    1514 1514 
         }
    1515 1515 
       },
    1516 1516
    1517 
    +  /**
    1518 
    +   * Determines whether the given "placesContext" menu item would open a link
    1519 
    +   * under some special conditions, but those special conditions cannot be met.
    1520 
    +   *
    1521 
    +   * @param {Element} item The menu or menu item to decide for.
    1522 
    +   *
    1523 
    +   * @returns {boolean} Whether the item is an "open" item that should be
    1524 
    +   *   hidden.
    1525 
    +   */
    1526 
    +  shouldHideOpenMenuItem(item) {
    1527 
    +    if (
    1528 
    +      item.hasAttribute("hide-if-disabled-private-browsing") &&
    1529 
    +      !lazy.PrivateBrowsingUtils.enabled
    1530 
    +    ) {
    1531 
    +      return true;
    1532 
    +    }
    1533 
    +
    1534 
    +    if (
    1535 
    +      item.hasAttribute("hide-if-private-browsing") &&
    1536 
    +      lazy.PrivateBrowsingUtils.isWindowPrivate(item.ownerGlobal)
    1537 
    +    ) {
    1538 
    +      return true;
    1539 
    +    }
    1540 
    +
    1541 
    +    if (
    1542 
    +      item.hasAttribute("hide-if-usercontext-disabled") &&
    1543 
    +      !Services.prefs.getBoolPref("privacy.userContext.enabled", false)
    1544 
    +    ) {
    1545 
    +      return true;
    1546 
    +    }
    1547 
    +
    1548 
    +    return false;
    1549 
    +  },
    1550 
    +
    1517 1551 
       async managedPlacesContextShowing(event) {
    1518 1552 
         let menupopup = event.target;
    1519 1553 
         let document = menupopup.ownerDocument;
    ... ... @@ -1528,12 +1562,6 @@ export var PlacesUIUtils = {
    1528 1562 
             menupopup.triggerNode.menupopup
    1529 1563 
           );
    1530 1564 
         }
    1531 
    -    let linkItems = [
    1532 
    -      "placesContext_open:newtab",
    1533 
    -      "placesContext_open:newwindow",
    1534 
    -      "placesContext_openSeparator",
    1535 
    -      "placesContext_copy",
    1536 
    -    ];
    1537 1565 
         // Hide everything. We'll unhide the things we need.
    1538 1566 
         Array.from(menupopup.children).forEach(function (child) {
    1539 1567 
           child.hidden = true;
    ... ... @@ -1555,12 +1583,18 @@ export var PlacesUIUtils = {
    1555 1583 
           openContainerInTabs_menuitem.disabled = !openContainerInTabs;
    1556 1584 
           openContainerInTabs_menuitem.hidden = false;
    1557 1585 
         } else {
    1558 
    -      linkItems.forEach(id => (document.getElementById(id).hidden = false));
    1559 
    -      document.getElementById("placesContext_open:newprivatewindow").hidden =
    1560 
    -        lazy.PrivateBrowsingUtils.isWindowPrivate(window) ||
    1561 
    -        !lazy.PrivateBrowsingUtils.enabled;
    1562 
    -      document.getElementById("placesContext_open:newcontainertab").hidden =
    1563 
    -        !Services.prefs.getBoolPref("privacy.userContext.enabled", false);
    1586 
    +      for (let id of [
    1587 
    +        "placesContext_open:newtab",
    1588 
    +        "placesContext_open:newcontainertab",
    1589 
    +        "placesContext_open:newwindow",
    1590 
    +        "placesContext_open:newprivatewindow",
    1591 
    +      ]) {
    1592 
    +        let item = document.getElementById(id);
    1593 
    +        item.hidden = this.shouldHideOpenMenuItem(item);
    1594 
    +      }
    1595 
    +      for (let id of ["placesContext_openSeparator", "placesContext_copy"]) {
    1596 
    +        document.getElementById(id).hidden = false;
    1597 
    +      }
    1564 1598 
         }
    1565 1599
    1566 1600 
         event.target.ownerGlobal.updateCommands("places");

  • browser/components/places/content/controller.js
    ... ... @@ -465,17 +465,7 @@ PlacesController.prototype = {
    465 465 
        *          and the item can be displayed, false otherwise.
    466 466 
        */
    467 467 
       _shouldShowMenuItem(aMenuItem, aMetaData) {
    468 
    -    if (
    469 
    -      aMenuItem.hasAttribute("hide-if-private-browsing") &&
    470 
    -      !PrivateBrowsingUtils.enabled
    471 
    -    ) {
    472 
    -      return false;
    473 
    -    }
    474 
    -
    475 
    -    if (
    476 
    -      aMenuItem.hasAttribute("hide-if-usercontext-disabled") &&
    477 
    -      !Services.prefs.getBoolPref("privacy.userContext.enabled", false)
    478 
    -    ) {
    468 
    +    if (PlacesUIUtils.shouldHideOpenMenuItem(aMenuItem)) {
    479 469 
           return false;
    480 470 
         }
    481 471
    ... ... @@ -566,8 +556,12 @@ PlacesController.prototype = {
    566 556 
        *     menuitem when there's no insertion point. An insertion point represents
    567 557 
        *     a point in the view where a new item can be inserted.
    568 558 
        *  9) The boolean `hide-if-private-browsing` attribute may be set to hide a
    569 
    -   *     menuitem in private browsing mode
    570 
    -   * 10) The boolean `hide-if-single-click-opens` attribute may be set to hide a
    559 
    +   *     menuitem in private browsing mode.
    560 
    +   * 10) The boolean `hide-if-disabled-private-browsing` attribute may be set to
    561 
    +   *     hide a menuitem if private browsing is not enabled.
    562 
    +   * 11) The boolean `hide-if-usercontext-disabled` attribute may be set to
    563 
    +   *     hide a menuitem if containers are disabled.
    564 
    +   * 12) The boolean `hide-if-single-click-opens` attribute may be set to hide a
    571 565 
        *     menuitem in views opening entries with a single click.
    572 566 
        *
    573 567 
        * @param {object} aPopup
    ... ... @@ -593,9 +587,6 @@ PlacesController.prototype = {
    593 587 
               item.getAttribute("hide-if-no-insertion-point") == "true" &&
    594 588 
               noIp &&
    595 589 
               !(ip && ip.isTag && item.id == "placesContext_paste");
    596 
    -        let hideIfPrivate =
    597 
    -          item.getAttribute("hide-if-private-browsing") == "true" &&
    598 
    -          PrivateBrowsingUtils.isWindowPrivate(window);
    599 590 
             // Hide `Open` if the primary action on click is opening.
    600 591 
             let hideIfSingleClickOpens =
    601 592 
               item.getAttribute("hide-if-single-click-opens") == "true" &&
    ... ... @@ -610,7 +601,6 @@ PlacesController.prototype = {
    610 601
    611 602 
             let shouldHideItem =
    612 603 
               hideIfNoIP ||
    613 
    -          hideIfPrivate ||
    614 604 
               hideIfSingleClickOpens ||
    615 605 
               hideIfNotSearch ||
    616 606 
               !this._shouldShowMenuItem(item, metadata);

  • browser/components/places/content/placesContextMenu.inc.xhtml
    ... ... @@ -52,13 +52,14 @@
    52 52 
                 command="placesCmd_open:window"
    53 53 
                 data-l10n-id="places-open-in-window"
    54 54 
                 selection-type="single"
    55 
    -            node-type="link"/>
    55 
    +            node-type="link"
    56 
    +            hide-if-private-browsing="true"/>
    56 57 
       <menuitem id="placesContext_open:newprivatewindow"
    57 58 
                 command="placesCmd_open:privatewindow"
    58 59 
                 data-l10n-id="places-open-in-private-window"
    59 60 
                 selection-type="single"
    60 61 
                 node-type="link"
    61 
    -            hide-if-private-browsing="true"/>
    62 
    +            hide-if-disabled-private-browsing="true"/>
    62 63 
       <menuitem id="placesContext_showInFolder"
    63 64 
                 data-l10n-id="places-show-in-folder"
    64 65 
                 command="placesCmd_showInFolder"

  • browser/components/places/tests/browser/browser_bookmark_context_menu_contents.js
    ... ... @@ -814,3 +814,118 @@ add_task(async function test_library_noselection_contextmenu_contents() {
    814 814 
         );
    815 815 
       });
    816 816 
     });
    817 
    +
    818 
    +add_task(async function test_private_browsing_window() {
    819 
    +  // Test the context menu when in a private browsing window.
    820 
    +
    821 
    +  let win = await BrowserTestUtils.openNewBrowserWindow({
    822 
    +    private: true,
    823 
    +  });
    824 
    +
    825 
    +  let optionItems = [
    826 
    +    "placesContext_open:newtab",
    827 
    +    // Hidden in private window "placesContext_open:newcontainertab"
    828 
    +    // Hidden in private window "placesContext_open:newwindow"
    829 
    +    "placesContext_open:newprivatewindow",
    830 
    +    "placesContext_show_bookmark:info",
    831 
    +    "placesContext_deleteBookmark",
    832 
    +    "placesContext_cut",
    833 
    +    "placesContext_copy",
    834 
    +    "placesContext_paste_group",
    835 
    +    "placesContext_new:bookmark",
    836 
    +    "placesContext_new:folder",
    837 
    +    "placesContext_new:separator",
    838 
    +  ];
    839 
    +
    840 
    +  // Test toolbar.
    841 
    +  await checkContextMenu(
    842 
    +    async function () {
    843 
    +      let toolbarBookmark = await PlacesUtils.bookmarks.insert({
    844 
    +        parentGuid: PlacesUtils.bookmarks.toolbarGuid,
    845 
    +        title: "Bookmark Title",
    846 
    +        url: TEST_URL,
    847 
    +      });
    848 
    +
    849 
    +      let toolbarNode = getToolbarNodeForItemGuid(toolbarBookmark.guid, win);
    850 
    +
    851 
    +      let contextMenu = win.document.getElementById("placesContext");
    852 
    +      let popupShownPromise = BrowserTestUtils.waitForEvent(
    853 
    +        contextMenu,
    854 
    +        "popupshown"
    855 
    +      );
    856 
    +
    857 
    +      EventUtils.synthesizeMouseAtCenter(
    858 
    +        toolbarNode,
    859 
    +        { button: 2, type: "contextmenu" },
    860 
    +        win
    861 
    +      );
    862 
    +      await popupShownPromise;
    863 
    +      return contextMenu;
    864 
    +    },
    865 
    +    [
    866 
    +      ...optionItems,
    867 
    +      "placesContext_showAllBookmarks",
    868 
    +      "toggle_PersonalToolbar",
    869 
    +      "show-other-bookmarks_PersonalToolbar",
    870 
    +    ],
    871 
    +    win.document
    872 
    +  );
    873 
    +
    874 
    +  // Test side bar.
    875 
    +  await withSidebarTree(
    876 
    +    "bookmarks",
    877 
    +    async tree => {
    878 
    +      await checkContextMenu(
    879 
    +        async bookmark => {
    880 
    +          tree.selectItems([bookmark.guid]);
    881 
    +
    882 
    +          let contextMenu =
    883 
    +            win.SidebarController.browser.contentDocument.getElementById(
    884 
    +              "placesContext"
    885 
    +            );
    886 
    +          let popupShownPromise = BrowserTestUtils.waitForEvent(
    887 
    +            contextMenu,
    888 
    +            "popupshown"
    889 
    +          );
    890 
    +          synthesizeClickOnSelectedTreeCell(tree, { type: "contextmenu" }, win);
    891 
    +          await popupShownPromise;
    892 
    +          return contextMenu;
    893 
    +        },
    894 
    +        optionItems,
    895 
    +        win.SidebarController.browser.contentDocument
    896 
    +      );
    897 
    +    },
    898 
    +    win
    899 
    +  );
    900 
    +
    901 
    +  // Test library window opened when using private browsing window.
    902 
    +  optionItems.splice(
    903 
    +    optionItems.indexOf("placesContext_show_bookmark:info"),
    904 
    +    1
    905 
    +  );
    906 
    +  optionItems.splice(0, 0, "placesContext_open");
    907 
    +
    908 
    +  await withLibraryWindow(
    909 
    +    "BookmarksToolbar",
    910 
    +    async ({ right }) => {
    911 
    +      await checkContextMenu(
    912 
    +        async bookmark => {
    913 
    +          let contextMenu = right.ownerDocument.getElementById("placesContext");
    914 
    +          let popupShownPromise = BrowserTestUtils.waitForEvent(
    915 
    +            contextMenu,
    916 
    +            "popupshown"
    917 
    +          );
    918 
    +          right.selectItems([bookmark.guid]);
    919 
    +          synthesizeClickOnSelectedTreeCell(right, { type: "contextmenu" });
    920 
    +          await popupShownPromise;
    921 
    +          return contextMenu;
    922 
    +        },
    923 
    +        optionItems,
    924 
    +        right.ownerDocument
    925 
    +      );
    926 
    +    },
    927 
    +    win
    928 
    +  );
    929 
    +
    930 
    +  await BrowserTestUtils.closeWindow(win);
    931 
    +});

  • browser/components/places/tests/browser/head.js
    ... ... @@ -6,8 +6,8 @@ ChromeUtils.defineLazyGetter(this, "gFluentStrings", function () {
    6 6 
       return new Localization(["branding/brand.ftl", "browser/browser.ftl"], true);
    7 7 
     });
    8 8
    9 
    -function openLibrary(callback, aLeftPaneRoot) {
    10 
    -  let library = window.openDialog(
    9 
    +function openLibrary(callback, aLeftPaneRoot, win = window) {
    10 
    +  let library = win.openDialog(
    11 11 
         "chrome://browser/content/places/places.xhtml",
    12 12 
         "",
    13 13 
         "chrome,toolbar=yes,dialog=no,resizable",
    ... ... @@ -27,10 +27,12 @@ function openLibrary(callback, aLeftPaneRoot) {
    27 27 
      *
    28 28 
      * @param {object} aLeftPaneRoot
    29 29 
      *        Hierarchy to open and select in the left pane.
    30 
    + * @param {Window} [win]
    31 
    + *        The window to use to open the library.
    30 32 
      * @returns {Promise}
    31 33 
      *          Resolves to the handle to the library window.
    32 34 
      */
    33 
    -function promiseLibrary(aLeftPaneRoot) {
    35 
    +function promiseLibrary(aLeftPaneRoot, win = window) {
    34 36 
       return new Promise(resolve => {
    35 37 
         let library = Services.wm.getMostRecentWindow("Places:Organizer");
    36 38 
         if (library && !library.closed) {
    ... ... @@ -42,7 +44,7 @@ function promiseLibrary(aLeftPaneRoot) {
    42 44 
           checkLibraryPaneVisibility(library, aLeftPaneRoot);
    43 45 
           resolve(library);
    44 46 
         } else {
    45 
    -      openLibrary(resolve, aLeftPaneRoot);
    47 
    +      openLibrary(resolve, aLeftPaneRoot, win);
    46 48 
         }
    47 49 
       });
    48 50 
     }
    ... ... @@ -380,9 +382,11 @@ function fillBookmarkTextField(id, text, win, blur = true) {
    380 382 
      * @param {Function} taskFn
    381 383 
      *        The task to execute once the sidebar is ready. Will get the Places
    382 384 
      *        tree view as input.
    385 
    + * @param {Window} [win]
    386 
    + *        The window to open the sidebar in, else uses the default window.
    383 387 
      */
    384 
    -var withSidebarTree = async function (type, taskFn) {
    385 
    -  let sidebar = document.getElementById("sidebar");
    388 
    +var withSidebarTree = async function (type, taskFn, win = window) {
    389 
    +  let sidebar = win.document.getElementById("sidebar");
    386 390 
       info("withSidebarTree: waiting sidebar load");
    387 391 
       let sidebarLoadedPromise = new Promise(resolve => {
    388 392 
         sidebar.addEventListener(
    ... ... @@ -395,7 +399,7 @@ var withSidebarTree = async function (type, taskFn) {
    395 399 
       });
    396 400 
       let sidebarId =
    397 401 
         type == "bookmarks" ? "viewBookmarksSidebar" : "viewHistorySidebar";
    398 
    -  SidebarController.show(sidebarId);
    402 
    +  win.SidebarController.show(sidebarId);
    399 403 
       await sidebarLoadedPromise;
    400 404
    401 405 
       let treeId = type == "bookmarks" ? "bookmarks-view" : "historyTree";
    ... ... @@ -406,7 +410,7 @@ var withSidebarTree = async function (type, taskFn) {
    406 410 
       try {
    407 411 
         await taskFn(tree);
    408 412 
       } finally {
    409 
    -    SidebarController.hide();
    413 
    +    win.SidebarController.hide();
    410 414 
       }
    411 415 
     };
    412 416
    ... ... @@ -419,9 +423,11 @@ var withSidebarTree = async function (type, taskFn) {
    419 423 
      * @param {Function} taskFn
    420 424 
      *        The task to execute once the Library is ready.
    421 425 
      *        Will get { left, right } trees as argument.
    426 
    + * @param {Window} [win]
    427 
    + *        The window to use to open the library.
    422 428 
      */
    423 
    -var withLibraryWindow = async function (hierarchy, taskFn) {
    424 
    -  let library = await promiseLibrary(hierarchy);
    429 
    +var withLibraryWindow = async function (hierarchy, taskFn, win = window) {
    430 
    +  let library = await promiseLibrary(hierarchy, win);
    425 431 
       let left = library.document.getElementById("placesList");
    426 432 
       let right = library.document.getElementById("placeContent");
    427 433 
       info("withLibrary: executing the task");
    ... ... @@ -477,8 +483,8 @@ function promisePopupHidden(popup) {
    477 483 
     }
    478 484
    479 485 
     // Identify a bookmark node in the Bookmarks Toolbar by its guid.
    480 
    -function getToolbarNodeForItemGuid(itemGuid) {
    481 
    -  let children = document.getElementById("PlacesToolbarItems").childNodes;
    486 
    +function getToolbarNodeForItemGuid(itemGuid, win = window) {
    487 
    +  let children = win.document.getElementById("PlacesToolbarItems").childNodes;
    482 488 
       for (let child of children) {
    483 489 
         if (itemGuid === child._placesNode.bookmarkGuid) {
    484 490 
           return child;

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.torproject.org. Manage all notifications · Help 

    _______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits