Closed Bug 1300755 Opened 8 years ago Closed 8 years ago

Notification anchors are not hidden when the user types in the location bar

Categories

(Firefox :: Site Identity, defect, P2)

Product:
Firefox
Component:
Site Identity
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 53
Iteration:
53.3 - Dec 26
Tracking Flags:
Tracking Status
firefox48 --- unaffected
firefox49 --- wontfix
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- verified

People

(Reporter: johannh, Assigned: Paolo)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [fxprivacy])

Attachments

(2 files, 2 obsolete files)

Example
54.01 KB, image/gif
Details
Bug 1300755 - Notification anchors are not hidden when the user types in the location bar.
58 bytes, text/x-review-board-request
past
: review+
Details
Attached image Example — 
See the appended GIF. All other icons are hidden, but the notification anchor stays. Not sure if we want this.
Summary: Notification anchors are not hidden when the searchbar is focused → Notification anchors are not hidden when the user types in the location bar
I'm not sure if we want to keep it or change it, but the current behavior makes sense to me. The icons that are hidden are tied to the location, which changes when the user types in the location bar. The other icons are tied to the page, which is still displayed until the user starts loading something else.
So I personally think we should hide them, it looks more consistent visually and doesn't make the user think about why certain icons were hidden and others not.

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> I'm not sure if we want to keep it or change it, but the current behavior
> makes sense to me. The icons that are hidden are tied to the location, which
> changes when the user types in the location bar. The other icons are tied to
> the page, which is still displayed until the user starts loading something
> else.

I don't understand the difference between tied to location and tied to page tbh. In my view the lock and the tracking protection icon have the same scope as the permission anchors.
I think I agree with Johann on this and moreover we should also change the (i) icon to lose the dot when typing in a site with granted permissions for the same reasons.
I'm on the fence about the animated sharing indicator though: consistency dictates that it should switch to the non-animated (i) for as long as the user is editing the location, but perhaps reminding about the privacy-invasive status should trump consistency in this case.
Not a blocker for 49. NI UX for insight.
Flags: needinfo?(philipp)
Priority: -- → P2
Whiteboard: [fxprivacy] [triage] → [fxprivacy][triage]
To elaborate a bit on the request, I'm told that in older versions of Firefox with the "carrot" container for permission anchors, these icons are still present when the user starts typing a new URL. It's only the lock and the shield that are hidden. I don't know the reason for this behavior, as I presume we hide the other icons as they are no longer relevant in the new URL the user is typing. Perhaps it was deemed less important to hide the permission icons when the (i) is right next to the cursor? Or it may just be a bug, I don't know.

The question is do we want to maintain that behavior now that the permission icons are between the (i) and the cursor, or do we want to hide everything? (See also comment 3 and comment 4 for an expanded definition of "everything").
Too late for 49, but we could still take changes for 50.
status-firefox49: affected → wontfix
(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> The other icons are tied to
> the page, which is still displayed until the user starts loading something
> else.

Actually, some other icons may be independent from the page: eg. e10s (hopefully gone soon if not already gone), add-on install.
I agree that the icons should be hidden when the user starts editing. Changing the URL moves the focus from the site that the user is currently on to the site that the user wants to visit.

(In reply to Panos Astithas [:past] from comment #4)
> I'm on the fence about the animated sharing indicator though: consistency
> dictates that it should switch to the non-animated (i) for as long as the
> user is editing the location, but perhaps reminding about the
> privacy-invasive status should trump consistency in this case.

That's an interesting case... In some ways, the above argument about focus is even more relevant here, since the animated icon draws more attention. But really, if the user is already about to navigate away, the concern about privacy might be moot since it takes longer to click the icon and remove the pref than to just perform the navigation.

There's another thing to consider though: when the user enter something in the URL bar and then clicks out of it, the icons remain hidden. There is an argument to be made to re-surface the icons in that case, since the users' focus has shifted back to the page.
Flags: needinfo?(philipp)
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
This patch fixes the regression that was originally described, since most people seem to agree this is not wanted. The related points brought up by Panos and Philipp are still to be discussed and I wanted to keep this fix separate, also so that we can land it soon.

We might want to speed it up to 50, what does everyone think?
Flags: qe-verify+
Comment on attachment 8791189 [details]
Bug 1300755 - Hide notification anchors on invalid pageproxystate.

https://reviewboard.mozilla.org/r/78694/#review77288

Hiding the notification icons even when the urlbar isn't focused doesn't feel right to me.

Steps to reproduce the issue I see:
1. Load data:text/html,<iframe src="https://people.mozilla.org/~fqueze2/webrtc/" height=300></iframe><iframe src="https://people.mozilla.org/~fqueze2/webrtc/" height=300></iframe>
2. Click "Video" in the first iframe (do not answer the prompt).
3. Focus the location bar (Cmd+L) and either clear it (backspace) or start typing.
4. Click somewhere in the document.

At this point the notification icons (the prompt for the camera) is impossible to find.

5. Click the "Video" button in the second iframe.

See a new prompt hanging from nowhere.
Attachment #8791189 - Flags: review?(florian) → review-
I think we already have consensus on hiding everything on edit and bringing it back on a blur event. That includes reverting the dot in the (i) and the animated sharing indicator.
Alright, I'm working on a) getting the identity block to show on urlbar blur b) getting the (i) icon situation working correctly, hiding everything on pageproxystate invalid and restoring to normal on valid.

We should rename pageproxystate.
Comment on attachment 8791189 [details]
Bug 1300755 - Hide notification anchors on invalid pageproxystate.

https://reviewboard.mozilla.org/r/78694/#review77596

r=me, but it's not a very confident r+. I haven't found any issue when looking at the code, and I tested it without spotting any undesirable behaviors, but I'm not very familiar with the code you are modifying, so my review may miss some issues.

::: browser/base/content/browser.js:2405
(Diff revision 2)
>  
> -    valid = !isBlankPageURL(uri.spec);
> +    valid = true;
>    }
>  
>    gURLBar.value = value;
>    gURLBar.valueIsTyped = !valid;

Looks like we don't need the 'valid' variable anymore, and you can just set this to !!value, and then use gURLBar.valueIsTyped for the SetPageProxyState call.

::: browser/themes/shared/identity-block/identity-block.inc.css:181
(Diff revision 2)
>  
> -#urlbar[pageproxystate="valid"] > #identity-box.verifiedDomain > #connection-icon,
> -#urlbar[pageproxystate="valid"] > #identity-box.verifiedIdentity > #connection-icon,
> -#urlbar[pageproxystate="valid"] > #identity-box.mixedActiveBlocked > #connection-icon {
> +#identity-box.verifiedDomain > #connection-icon,
> +#identity-box.verifiedIdentity > #connection-icon,
> +#identity-box.mixedActiveBlocked > #connection-icon {
>    list-style-image: url(chrome://browser/skin/identity-secure.svg);
>    visibility: visible;

Given that the visibility: collapse applies only when pageproxystate is "invalid", do we really need this visibility: visible rule here? (it's a real question, not a request for a change phrased as a question)
Attachment #8791189 - Flags: review?(florian) → review+
(In reply to Florian Quèze [:florian] [:flo] from comment #16) 
> ::: browser/themes/shared/identity-block/identity-block.inc.css:181
> (Diff revision 2)
> >  
> > -#urlbar[pageproxystate="valid"] > #identity-box.verifiedDomain > #connection-icon,
> > -#urlbar[pageproxystate="valid"] > #identity-box.verifiedIdentity > #connection-icon,
> > -#urlbar[pageproxystate="valid"] > #identity-box.mixedActiveBlocked > #connection-icon {
> > +#identity-box.verifiedDomain > #connection-icon,
> > +#identity-box.verifiedIdentity > #connection-icon,
> > +#identity-box.mixedActiveBlocked > #connection-icon {
> >    list-style-image: url(chrome://browser/skin/identity-secure.svg);
> >    visibility: visible;
> 
> Given that the visibility: collapse applies only when pageproxystate is
> "invalid", do we really need this visibility: visible rule here? (it's a
> real question, not a request for a change phrased as a question)

Yeah I initially thought the same thing but it seems that the icon should only be shown if these classes are applied and there are cases when there's none of these classes on the identity-box. So afaik we need it.
Iteration: --- → 52.1 - Oct 3
Whiteboard: [fxprivacy] → [fxprivacy][triage]
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #9)
> There's another thing to consider though: when the user enter something in
> the URL bar and then clicks out of it, the icons remain hidden. There is an
> argument to be made to re-surface the icons in that case, since the users'
> focus has shifted back to the page.

On the other hand, exactly because the focus of the user has moved away, the absence of security icons (or "invalid" page proxy state like it's called in the code) is relevant when the attention moves back to the location bar. It reminds the user that the address they see is not the address of the page that they are upon, and they have to press ESC to see the original address of the page.

So I'm not sure whether showing the security icons near a URL to which they don't apply is desirable.

(As a side note, maybe while editing an existing non-blank URL the dimmed "i" icon should be a dimmed "x" icon and clicking it could be used to revert the intent to navigate to a different URL. The ESC key is not very discoverable.)
On the Elm branch, we may need a related test to ensure that persistent popup notifications are reopened after the page proxy state is back to valid.

In Nightly, we need to test that popup notifications are still displayed correctly if they are opened while the page proxy state is invalid, that is while the user is editing the URL to navigate. We may need to wait until the user has finished editing before we can open the popup notification panel, or anchor it somewhere else until the edit is reverted.
Iteration: 52.1 - Oct 3 → ---
Whiteboard: [fxprivacy][triage] → [fxprivacy]
Flags: needinfo?(philipp)
(In reply to :Paolo Amadini from comment #23)

> On the other hand, exactly because the focus of the user has moved away, the
> absence of security icons (or "invalid" page proxy state like it's called in
> the code) is relevant when the attention moves back to the location bar. It
> reminds the user that the address they see is not the address of the page
> that they are upon, and they have to press ESC to see the original address
> of the page.
> 
> So I'm not sure whether showing the security icons near a URL to which they
> don't apply is desirable.

Even if we don't show security icons when the url doesn't match the page actually loaded, I think we still need to show sharing (camera, etc...) indicators whenever the location bar isn't focused.
(In reply to :Paolo Amadini from comment #23)
> (In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from
> comment #9)
> > There's another thing to consider though: when the user enter something in
> > the URL bar and then clicks out of it, the icons remain hidden. There is an
> > argument to be made to re-surface the icons in that case, since the users'
> > focus has shifted back to the page.
> 
> On the other hand, exactly because the focus of the user has moved away, the
> absence of security icons (or "invalid" page proxy state like it's called in
> the code) is relevant when the attention moves back to the location bar. It
> reminds the user that the address they see is not the address of the page
> that they are upon, and they have to press ESC to see the original address
> of the page.

No, it doesn't remind the user to press ESC, because we have to assume that there are users (I actually believe the vast majority of users) that don't know about this way to get the security state back. It's even more complicated than just pressing ESC, they have to click inside the URL bar again and press ESC. I don't think I could get any of my relatives to bring the site info back after they clicked outside the URL bar. :)

> 
> So I'm not sure whether showing the security icons near a URL to which they
> don't apply is desirable.
> 

I don't see the problem with that. I don't think anyone except us thinks about pageproxystate when seeing their URL bar to determine if the URL is currently correct. I also can't make up an attack vector that involves the user typing a wrong address in their URL bar.

Not showing the security indicators on urlbar blur makes things more insecure, because we effectively remove them for any user who doesn't know about the click-again-and-press-ESC trick. An (x) button could do it but if this is really not a security risk (which I believe it isn't) why add an additional UI element?
This patch makes tests pass again (let's wait for another try, had to do a huge rebase) and gets rid of the "pageproxystate" concept, splitting it into an "editing" attribute, a .blankPage class and a field for the original URL. Pageproxystate doesn't really fit anymore, since with the proposed changes the URL bar could show an "invalid" URL without losing the security indicators. The first patch already did that.

Paolo, I wanted to make sure you're happy with the changes around this so I requested you for review. We might also wait for Philipps input.

Thanks :)
(In reply to Johann Hofmann [:johannh] - partially unresponsive until 11/14 from comment #26)
> I don't see the problem with that. I don't think anyone except us thinks
> about pageproxystate when seeing their URL bar to determine if the URL is
> currently correct. I also can't make up an attack vector that involves the
> user typing a wrong address in their URL bar.

I actually agree with this analysis, I guess it's time to change how we treat this case.

(In reply to Johann Hofmann [:johannh] - partially unresponsive until 11/14 from comment #29)
> gets rid of the "pageproxystate" concept, splitting it into
> an "editing" attribute, a .blankPage class and a field for the original URL.

Even before I looked at the patch, this sounded great!

Gijs, can you just take a quick look at the patches on the bug and see if we are missing something obvious? While I'm familiar with the identity block logic I'm not familiar with the editing logic of the URL bar in browser.js. The changes look straightforward to me but it's a delicate part of the code.
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Johann Hofmann [:johannh] - partially unresponsive until 11/14 from comment #26)
> I don't see the problem with that. I don't think anyone except us thinks
> about pageproxystate when seeing their URL bar to determine if the URL is
> currently correct. I also can't make up an attack vector that involves the
> user typing a wrong address in their URL bar.

It's not very difficult to convince a user to drag a URL to the location bar, and to change that URL and use that for spoofing.

I'm not thrilled about removing a clear indicator of whether the text in the location bar is what the user typed or a reflection of the current page contents, and Paolo's comment doesn't give me enough context to understand why this is necessary or how it relates to this bug - there seems to already be a separate patch to accomplish what we want without removing pageproxystate?
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8791189 - Flags: review+
Comment on attachment 8798167 [details]
Bug 1300755 - Rename pageproxystate to editing and separate from blank page state.

Following up to comment 32, we should probably split the behavior change about the case where the URL is being edited and the location bar is not focused to a different bug. This means reworking the first patch too.

Also we need to write the tests that I mentioned in comment 24, because we always have to handle the case where we show popup notifications while the URL bar is focused and the "pageproxystate" is "invalid". I guess what we want to do in that case is to delay those notifications until the anchors are visible, and hide any permanent notification when the anchors are hidden, analogously to what we do when the user switches tabs.
Attachment #8798167 - Flags: review?(paolo.mozmail)
I'm afraid that we won't reach a perfect solution unless we start resetting the text in the URL bar when the user de-focuses it (and restoring the input value once the focus moves back). Such a change would require a wider discussion in a different bug since it undoubtedly will open up all kinds of other edge cases.

Until then, I'd prefer to proceed as suggested and hide the anchors while editing the URL.
Flags: needinfo?(philipp)
Panos, given Johann's partially unresponsive status, is there someone else that could implement Philipp's suggestion in comment 34?
Flags: needinfo?(past)
Paolo, can you pick this one up since you have already reviewed it?
Flags: needinfo?(past) → needinfo?(paolo.mozmail)
Assignee: jhofmann → paolo.mozmail
Flags: needinfo?(paolo.mozmail)
Iteration: --- → 52.3 - Nov 7
we are a week away from RC, too late, this is now a wontfix for 50.
status-firefox50: affected → wontfix
Attachment #8791189 - Attachment is obsolete: true
Attachment #8798167 - Attachment is obsolete: true
This new version of the patch is limited to just changing the visibility of the anchors, while also moving the popup to the identity icon instead of the current tab in the tabstrip when no anchor is visible.

We could avoid hardcoding "identity-icon" by specifying the fallback anchor indirectly through an attribute on the browser, like we do for the default anchor which is a slightly different concept from the fallback anchor, but I'm not sure it's worth the complexity.

This still needs tests, and I still have to check existing failing tests to see if the change in meaning of _currentAnchorElement causes any regressions.
I had to change the code a bit to handle a few edge cases identified by regressions tests, for the rest most of the tests just had to be modified to use an actual page instead of about:blank.

I still need to write tests for invalid pageproxystate and the about:blank case.

I wonder if we should file a bug to also hide the notification if the awesomebar is open.
What you are doing is breaking bug 1231025 even more.
Apparently, UX team haven't ever read that bug... Please read it.
I filed that bug for them to make a decision on what is the better way to remove the twitching, but instead I see more useless buttons being added to identity block, more twitching being implemented.
Depends on: 1231025
Flags: needinfo?(paolo.mozmail)
I've replied in bug 1231025 comment 5. Thanks!
Flags: needinfo?(paolo.mozmail)
Iteration: 52.3 - Nov 14 → 53.1 - Nov 28
Tryserver build, including all the previous notifications work:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=12335468ae342b6847202295e3c9c98eab1543f7
Comment on attachment 8808200 [details]
Bug 1300755 - Notification anchors are not hidden when the user types in the location bar.

https://reviewboard.mozilla.org/r/91062/#review93270

::: browser/base/content/test/popupNotifications/browser.ini:19
(Diff revision 3)
>  skip-if = (os == "linux" && (debug || asan))
>  [browser_popupNotification_5.js]
>  skip-if = (os == "linux" && (debug || asan))
>  [browser_popupNotification_checkbox.js]
>  skip-if = (os == "linux" && (debug || asan))
> +[browser_popupNotification_no_anchors.js]

It looks that you forgot to add this file and the try builds failed.
(In reply to Panos Astithas [:past] from comment #46)
> It looks that you forgot to add this file and the try builds failed.

Also, not very useful for review :-)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6968d5b00442a2ce0920ed736e44dfef94e873c1
There are still failures in the tests, likely due to the way we wait for browser load events.
Comment on attachment 8808200 [details]
Bug 1300755 - Notification anchors are not hidden when the user types in the location bar.

https://reviewboard.mozilla.org/r/91062/#review93838

Works as advertised, thanks!

::: browser/base/content/browser.css
(Diff revision 4)
>  #urlbar[pageproxystate="invalid"] > #identity-box {
>    pointer-events: none;
>  }
>  
> -#urlbar[pageproxystate="invalid"] > #identity-box > #notification-popup-box {
> -  pointer-events: auto;

Isn't this necessary any more? pointer-events is an inherited property and we reset it in the rule above.

::: browser/base/content/browser.js:2387
(Diff revision 4)
>    // We didn't find a matching window, so open a new one.
>    return openDialog("chrome://browser/content/pageinfo/pageInfo.xul", "",
>                      "chrome,toolbar,dialog=no,resizable", args);
>  }
>  
> -function URLBarSetURI(aURI) {
> +function URLBarSetURI(aURI, aOptions = {}) {

This needs a comment to describe the available options. aURI is pretty obvious, but aOptions isn't.

::: browser/base/content/browser.js:4493
(Diff revision 4)
>          this.reloadCommand.setAttribute("disabled", "true");
>        } else {
>          this.reloadCommand.removeAttribute("disabled");
>        }
>  
> -      URLBarSetURI(aLocationURI);
> +      URLBarSetURI(aLocationURI, { isForLocationChange: true });

Did you audit the tree for other call sites that need to be modified? I see about a dozen hits (mostly tests) and it's not clear from a quick glance which ones need to.
Attachment #8808200 - Flags: review?(past) → review+
Iteration: 53.1 - Nov 28 → 53.2 - Dec 12
(In reply to Panos Astithas [:past] from comment #50)
> > -#urlbar[pageproxystate="invalid"] > #identity-box > #notification-popup-box {
> > -  pointer-events: auto;
> 
> Isn't this necessary any more? pointer-events is an inherited property and
> we reset it in the rule above.

I'm not sure what this comment means. This rule re-enabled the pointer events on the notification anchors, which is not necessary anymore now since these are now hidden.

> ::: browser/base/content/browser.js:4493
> > +      URLBarSetURI(aLocationURI, { isForLocationChange: true });
> 
> Did you audit the tree for other call sites that need to be modified? I see
> about a dozen hits (mostly tests) and it's not clear from a quick glance
> which ones need to.

Only this place actually sets the URI, but I'd rather have an explicit option than use the presence of aURI to determine whether this was called in response to a location change.
Let's see if some other test changes actually solve the timeouts.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b655951fc5be97d783c0bdfd16b040949af771d8
Looks like browser_popupNotification_4.js is now failing constantly on Mac and Windows, it might be related to the existing intermittent bug 1239930. The test case opens a new window, I suspect we might not be waiting enough for the window to be fully initialized.
This is wrong, right?

https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/browser/base/content/test/popupNotifications/browser_popupNotification_5.js#193

I'd expect the popup to be hidden in the first window while the second window is selected. I suspect this check only succeeds accidentally because of current timing.
Flags: needinfo?(past)
The latest version has even more test refactoring to handle cases where we didn't wait enough when creating new windows or opening popup notifications. I'm not asking for review again since the only functional change is the removal of the check from comment 55, but the patch is now posted if you want to take a look.

Let's see if this solves the failures:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7ef2f30b76d329b622ede33ce54223ba27b5c0c1

These changes might also solve the intermittent bug 1239930.
Blocks: 1239930
(In reply to :Paolo Amadini from comment #55)
> I'd expect the popup to be hidden in the first window while the second
> window is selected. I suspect this check only succeeds accidentally because
> of current timing.

Why would you expect that? A separate window doesn't reuse the URL bar like a separate tab does, which is what requires us to hide the notification in that case. Try opening two windows manually and observing a notification in the one in the background: it doesn't go away, because, honestly, why should it?
Flags: needinfo?(past)
(In reply to Panos Astithas [:past] from comment #58)
> Why would you expect that? A separate window doesn't reuse the URL bar like
> a separate tab does, which is what requires us to hide the notification in
> that case. Try opening two windows manually and observing a notification in
> the one in the background: it doesn't go away, because, honestly, why should
> it?

Ah, I just assumed we hid panels on inactive windows because of possible z-order issues, but I see this isn't the case. What happens on Windows, maybe to work around these issues, is that the panel disappears momentarily when a new window is opened, to be displayed again soon.

This is perceivable when testing manually in the slower debug builds, and this is why the automated test fails. For some reason it stops running with the panel hidden in the original window, hence my wrong assumption.

I'll debug some more, in the worst case we may change what we check slightly, or skip the check only on Windows. The new try builds with this check removed have succeeded!
In that case I agree that keeping that check is really necessary. It verifies the current behavior, but from a UX POV I don't think we need to guarantee that the panel remains open.
> In that case I agree that keeping that check is really necessary.

Oops, I meant to say "_isn't_ really necessary".
Pushed by paolo.mozmail@amadzone.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1955f4507d13
Notification anchors are not hidden when the user types in the location bar. r=past
Thanks, I landed the current version without the check.
https://hg.mozilla.org/mozilla-central/rev/1955f4507d13
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Iteration: 53.2 - Dec 12 → 53.3 - Dec 26
Given the patch is huge and the issue has been there for a while, I'm going to won't fix this in Beta51.
status-firefox51: affected → wontfix
I have reproduced this bug with Nightly 51.0a1 (2016-09-06) on Windows 7, 64 Bit ! 

This bug's fix is verified with latest Nightly 

Build  ID  : 20161221030226
User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
[bugday-20161221]
Thanks!
Status: RESOLVED → VERIFIED
I reproduced this issue using Fx 51.0a1, build ID: 20160906030431, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 53.0a1, build ID:20161227030213, on Windows 10 x64, Ubuntu 14.04 LTS and Mac OS X 10.11.

Cheers!
status-firefox53: fixed → verified
Flags: qe-verify+
Looks like a lot of the hugeness of this patch is just test changes. How do you feel about requesting Aurora approval on this, Paolo?
Flags: needinfo?(paolo.mozmail)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #69)
> Looks like a lot of the hugeness of this patch is just test changes. How do
> you feel about requesting Aurora approval on this, Paolo?

The patch is not too complex except for the tests, but it's written on top of the changes we did early in Firefox 53, so it could require some non-trivial merging. Given that the state before the patch isn't too bad, I think we could wait one release and give this fix to users together with the Permission Notifications redesign.
Flags: needinfo?(paolo.mozmail)
Depends on: 1328304
Depends on: 1340538
Depends on: 1347627
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: