Jared Wein [:jaws] (please needinfo? me) Assignee
	Description • 11 years ago

      No description provided.

	Matthew N. [:MattN]
	Comment 1 • 11 years ago

Comment on attachment 778230 [details] [diff] [review]
895505.patch

Review of attachment 778230 [details] [diff] [review]:
-----------------------------------------------------------------

Might be a good idea to do a try push with mochitests as I wouldn't be surprised if a11y or b-c tests break (although I didn't see references to the IDs of the deleted elements).

	Matthew N. [:MattN]
	Comment 2 • 11 years ago

(Quoting Dão Gottwald [:dao] from bug 895650 comment #1)
> > (bonus points: what do we do with the backout and nightly?)
> 
> Right. And what about users that downgrade (to the ESR or the previous
> release) for some reason or another?

Will this also have an affect on people downgrading? Will be remove those elements from the placements/currentset if it doesn't exists? (I've heard discussion going both ways on this).

	Jared Wein [:jaws] (please needinfo? me) Assignee
	Comment 3 • 11 years ago

https://hg.mozilla.org/projects/ux/rev/4883dc0017a1

(In reply to Matthew N. [:MattN] from comment #2)
> (Quoting Dão Gottwald [:dao] from bug 895650 comment #1)
> > > (bonus points: what do we do with the backout and nightly?)
> > 
> > Right. And what about users that downgrade (to the ESR or the previous
> > release) for some reason or another?
> 
> Will this also have an affect on people downgrading? Will be remove those
> elements from the placements/currentset if it doesn't exists? (I've heard
> discussion going both ways on this).

When an item doesn't exist we leave it alone in currentset and just skip over it so I don't expect this to cause any issues.

(In reply to Matthew N. [:MattN] from comment #1)
> Comment on attachment 778230 [details] [diff] [review]
> 895505.patch
> 
> Review of attachment 778230 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Might be a good idea to do a try push with mochitests as I wouldn't be
> surprised if a11y or b-c tests break (although I didn't see references to
> the IDs of the deleted elements).

All green on Try, https://tbpl.mozilla.org/?tree=Try&rev=4b5524687d67

	Mike de Boer [:mikedeboer]
	Comment 4 • 11 years ago

bah, I can't believe I missed this. Well I kinda do: I had to merge browser.xul so many times during the course of bug 755598, that these two buttons must've crept back in, while I explicitly removed them originally.

In other words, since MattN did a try run before with a correct patch, you needn't had to go though all this trouble ;) Too late now!

Glad you fixed it.

	:Gijs (he/him)
	Comment 5 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/4883dc0017a1