This goes through the previous changes in the dependencies of bug 877389, and does two things:
1) Remove instances of \n
2) Change reporting of exceptions so that they are passed as separate arguments. This should result
in an improved display of the exception in the browser console, should it occur.
Differential Revision: https://phabricator.services.mozilla.com/D180843
Remove the `browser.bookmarks.editDialog.delayedApply.enabled` pref, and for all usages in conditional branches, assume that it is `true`. (Since we want to be using delayed apply logic going forward.)
`delayed_apply` spinoffs of tests are removed and merged with their instant apply counterparts. The only test removed without a corresponding spinoff is `browser_editBookmark_tags_liveUpdate.js`. That test is checking whether tags are updated in the edit dialog, if they are updated from a different place. The problem is that we don't really want this behavior anymore. It was previously set to only run with `browser.bookmarks.editDialog.delayedApply.enabled` set to `false`, but now that it's no longer configurable, it made most sense to simply get rid of this test.
https://treeherder.mozilla.org/jobs?revision=c0741eca62212a75a9dd52fc2c5c2b6c34f9f2d7&repo=try
Differential Revision: https://phabricator.services.mozilla.com/D176370
As the remaining places.properties strings are used only for
errors and other dialogs from contexts that require sync localization,
it's better to put them in their own new file placesPrompts.ftl.
Differential Revision: https://phabricator.services.mozilla.com/D176646
This also allows dropping the PlacesUIUtils getFormattedString() and getPluralString() functions as unused
Depends on D176643
Differential Revision: https://phabricator.services.mozilla.com/D176644
As expected, the try job flagged a bunch of test failures when flipping the default `delayedApply` pref to `true`. Some of these failures are legitimate issues:
- When creating a new folder in the tree under "Location", renaming the folder doesn't update its name in the Location field.
- When right clicking a bookmark in the sidebar, and creating a new folder, the folder doesn't get placed before the bookmark, i.e. the insertion point is ignored.
And as you pointed out, tags were being wiped out on blur in the Star/Toolbar panels. These issues have been fixed. The rest of the failures have been fixed in one of these ways:
- Update the test to pass regardless of `delayedApply` setting. This was the preferred method.
- Force the test to use instant apply. This was only done for tests that have an existing delayed apply counterpart.
- Force the entire test suite to use instant apply. This was only done for one file, namely `browser_bookmark_popup.js`. I'll file a bug to spin off a delayed apply version of this suite.
try job with `delayedApply` enabled: https://treeherder.mozilla.org/jobs?repo=try&revision=50e9cdb65feaec07c9519e928caf62042c3df9a4
try job with `delayedApply` disabled: https://treeherder.mozilla.org/jobs?repo=try&revision=1102b5076a79bf08a0e4b073fdf369af97a16ef7
Differential Revision: https://phabricator.services.mozilla.com/D168690
'fromChrome' really meant "force tabs to open in the foreground", so let's
rename it accordingly.
This removes the attempt to document arguments for openUILinkIn.
I'll add documentation back on the end of this stack, for openLinkIn, when
various bits are reorganized anyway.
Differential Revision: https://phabricator.services.mozilla.com/D170384
'fromChrome' really meant "force tabs to open in the foreground", so let's
rename it accordingly.
This removes the attempt to document arguments for openUILinkIn.
I'll add documentation back on the end of this stack, for openLinkIn, when
various bits are reorganized anyway.
Depends on D170210
Differential Revision: https://phabricator.services.mozilla.com/D170384
As expected, the try job flagged a bunch of test failures when flipping the default `delayedApply` pref to `true`. Some of these failures are legitimate issues:
- When creating a new folder in the tree under "Location", renaming the folder doesn't update its name in the Location field.
- When right clicking a bookmark in the sidebar, and creating a new folder, the folder doesn't get placed before the bookmark, i.e. the insertion point is ignored.
And as you pointed out, tags were being wiped out on blur in the Star/Toolbar panels. These issues have been fixed. The rest of the failures have been fixed in one of these ways:
- Update the test to pass regardless of `delayedApply` setting. This was the preferred method.
- Force the test to use instant apply. This was only done for tests that have an existing delayed apply counterpart.
- Force the entire test suite to use instant apply. This was only done for one file, namely `browser_bookmark_popup.js`. I'll file a bug to spin off a delayed apply version of this suite.
try job with `delayedApply` enabled: https://treeherder.mozilla.org/jobs?repo=try&revision=50e9cdb65feaec07c9519e928caf62042c3df9a4
try job with `delayedApply` disabled: https://treeherder.mozilla.org/jobs?repo=try&revision=1102b5076a79bf08a0e4b073fdf369af97a16ef7
Differential Revision: https://phabricator.services.mozilla.com/D168690
As expected, the try job flagged a bunch of test failures when flipping the default `delayedApply` pref to `true`. Some of these failures are legitimate issues:
- When creating a new folder in the tree under "Location", renaming the folder doesn't update its name in the Location field.
- When right clicking a bookmark in the sidebar, and creating a new folder, the folder doesn't get placed before the bookmark, i.e. the insertion point is ignored.
And as you pointed out, tags were being wiped out on blur in the Star/Toolbar panels. These issues have been fixed. The rest of the failures have been fixed in one of these ways:
- Update the test to pass regardless of `delayedApply` setting. This was the preferred method.
- Force the test to use instant apply. This was only done for tests that have an existing delayed apply counterpart.
- Force the entire test suite to use instant apply. This was only done for one file, namely `browser_bookmark_popup.js`. I'll file a bug to spin off a delayed apply version of this suite.
try job with `delayedApply` enabled: https://treeherder.mozilla.org/jobs?repo=try&revision=50e9cdb65feaec07c9519e928caf62042c3df9a4
try job with `delayedApply` disabled: https://treeherder.mozilla.org/jobs?repo=try&revision=1102b5076a79bf08a0e4b073fdf369af97a16ef7
Differential Revision: https://phabricator.services.mozilla.com/D168690
As expected, the try job flagged a bunch of test failures when flipping the default `delayedApply` pref to `true`. Some of these failures are legitimate issues:
- When creating a new folder in the tree under "Location", renaming the folder doesn't update its name in the Location field.
- When right clicking a bookmark in the sidebar, and creating a new folder, the folder doesn't get placed before the bookmark, i.e. the insertion point is ignored.
And as you pointed out, tags were being wiped out on blur in the Star/Toolbar panels. These issues have been fixed. The rest of the failures have been fixed in one of these ways:
- Update the test to pass regardless of `delayedApply` setting. This was the preferred method.
- Force the test to use instant apply. This was only done for tests that have an existing delayed apply counterpart.
- Force the entire test suite to use instant apply. This was only done for one file, namely `browser_bookmark_popup.js`. I'll file a bug to spin off a delayed apply version of this suite.
try job with `delayedApply` enabled: https://treeherder.mozilla.org/jobs?repo=try&revision=50e9cdb65feaec07c9519e928caf62042c3df9a4
try job with `delayedApply` disabled: https://treeherder.mozilla.org/jobs?repo=try&revision=1102b5076a79bf08a0e4b073fdf369af97a16ef7
Differential Revision: https://phabricator.services.mozilla.com/D168690
Most usage is a straight replacement but gtk needs extra changes as it transfers plain text in UTF8 natively and needs to be converted into UTF16, and Windows uses single-byte characters for RTF and CF_HTML formats so we preserve this.
Differential Revision: https://phabricator.services.mozilla.com/D158587
Ensure that we can safely load `editBookmark.js` as the `gEditItemOverlay` instance without breaking the library panel. As the library does not currently support OK/Cancel operation, this does not perform any "delayed apply" logic, we simply save on blur as usual.
The tricky part was getting bulk tagging to work. With the current design of the `BookmarkState` class, it only supports modifying a **single** bookmark, and it will break when trying to add a tag to multiple bookmarks at once. This is where we left off with the previous revision... I believe in this case, it makes sense to simply revert back to the logic we had in `instantEditBookmark.js` to perform the bulk tag operation.
https://treeherder.mozilla.org/jobs?repo=try&revision=a1f58a770b16edd519da6e0d55e2f3c529d8f8de
Differential Revision: https://phabricator.services.mozilla.com/D164250
Based on https://phabricator.services.mozilla.com/D141582. Updates the bookmark toolbar to delay creating new bookmarks and folders while "delayed apply" mode is enabled.
One edge case which is not currently accounted for, is the case where user clicks on "Bookmark All Tabs", and then creates folders from within the dialog box, for example:
{F4228947}
In this case, the new folders will be created instantly, even if delayed apply is enabled. Cancelling the dialog box will remove them. (This is why we still need to use "batching", even in delayed apply mode.)
Because of how the dialog is currently structured, it seems fairly complex to handle this case. I am not sure if it is within the scope of this project, open to advice.
Differential Revision: https://phabricator.services.mozilla.com/D160554
