986808 - Sidebar widget - Don't copy over <menuseparator> if it's the first item, or if previous item copied is also a menuseparator.

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[:Gijs (he/him)]

See bug 981305.

Comment 1

[:Gijs (he/him)]

Attachment: don't insert duplicate separators in sidebar widget,

I went on a unifying rampage. Not exactly the minimal fix, but as this only needs to head to Aurora and not Beta, it seems OK to me. Mike, look OK to you?

Comment 2

[Mike Conley (:mconley) (:⚙️) (PTO - Sept 15 - 19)]

Comment on attachment 8398148 [details] [diff] [review]
don't insert duplicate separators in sidebar widget,

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

Wow - nice job DRYing up this code! It might be nice to generalize some of this even further, and allow the Help subview and add-ons use this code as well, but I think this is a fine intermediate step.

Comment 3

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/a24bf53b82c6

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a24bf53b82c6

Comment 5

[:Gijs (he/him)]

Comment on attachment 8398148 [details] [diff] [review]
don't insert duplicate separators in sidebar widget,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis / sidebar widget
User impact if declined: sidebar widget subview might look sloppy, with stray separators in there
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): reasonably low, although there's a good bit of refactoring in here, it's baked for quite a while with no known issues, and there are several automated tests for the results of the code
String or IDL/UUID changes made by this patch: none

Comment 6

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/dcd2cadb740a

Comment 7

[u279076]

Flagging in-testsuite+ since this appears to have landed with tests.