Closed Bug 895724 Opened 7 years ago Closed 6 years ago

Remove reload-button and stop-button from browser.xul since they are not needed anymore

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: jaws, Assigned: jaws)

References

Details

(Whiteboard: [Australis:M8])

Attachments

(1 file)

Attached patch 895505.patchSplinter Review
No description provided.
Attachment #778230 - Flags: review?(mnoorenberghe+bmo)
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).
Attachment #778230 - Flags: review?(mnoorenberghe+bmo) → review+
(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).
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
Whiteboard: [Australis:M8][fixed-in-ux]
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.
https://hg.mozilla.org/mozilla-central/rev/4883dc0017a1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M8][fixed-in-ux] → [Australis:M8]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.