Fix places-related mochitests in chrome/ for async-transactions

RESOLVED FIXED in Firefox 56

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: standard8, Assigned: mak)

Tracking

unspecified
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
There are various tests in chrome/ that fail when we try to turn on async-transactions.

We should fix those.
(Assignee)

Comment 1

2 years ago
stealing bug.
Assignee: standard8 → mak77
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Blocks: 1377864
(Assignee)

Comment 3

2 years ago
Looks like there are some failures to look into.
(Assignee)

Updated

2 years ago
Attachment #8882723 - Flags: review?(standard8)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8882723 [details]
Bug 1376929 - Fix places-related mochitests in chrome/ for async-transactions.

https://reviewboard.mozilla.org/r/153816/#review159302
Attachment #8882723 - Flags: review?(standard8) → review+

Comment 7

2 years ago
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/1f5e741ab2b6
Fix places-related mochitests in chrome/ for async-transactions. r=standard8

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1f5e741ab2b6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Comment 9

2 years ago
> -    if (!aURI || !aTags || !Array.isArray(aTags)) {
> -      throw Cr.NS_ERROR_INVALID_ARG;
> +    if (!aURI || !aTags || !Array.isArray(aTags) || aTags.length == 0) {
> +      throw Components.Exception("Invalid value for tags", Cr.NS_ERROR_INVALID_ARG);

Why did you do that? An empty array is still an array. getTagsForURI can return an empty array; why am I not allowed to pass the returned value straight to tagURI?

This broke https://addons.mozilla.org/en-US/firefox/addon/bmreplace/.
Flags: needinfo?(mak77)
(Assignee)

Comment 10

2 years ago
(In reply to Dmitry Gutov from comment #9)
> Why did you do that? An empty array is still an array. getTagsForURI can
> return an empty array; why am I not allowed to pass the returned value
> straight to tagURI?

because trying to tag with an empty array may hide bugs in the code. Suppose some code is trying to add tags, but due to a bug it ends up passing an empty array every time.
Flags: needinfo?(mak77)

Comment 11

2 years ago
> Suppose some code is trying to add tags, but due to a bug it ends up passing an empty array every time.

That is silly. An imaginary bug that ends up generating an empty array unintentionally could just as well generate a non-empty array with undesired values.

This is a backward incompatible change. Please revert it.
(Assignee)

Comment 12

2 years ago
why? the extension can be fixed easily with a simple if, in a few minutes. I prefer having the API helping us finding unexpected behaviors in the UI code.

Comment 13

2 years ago
I doubt this is the only extension that will need to be fixed. And debugging this breakage took more than a few minutes.
You need to log in before you can comment on or make changes to this bug.