Closed Bug 907324 Opened 8 years ago Closed 8 years ago

Regression from bug 776446

Categories

(Firefox Graveyard :: Webapp Runtime, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: marco, Assigned: marco)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch remove_bar (obsolete) — Splinter Review
After bug 776446 we have a small bar on the bottom of apps' windows. The menu separator in the popupset is causing this.
We could just get rid of it for now and reintroduce it if/when we'll have a context menu with default entries.
Attachment #792982 - Flags: review?(myk)
See Also: → 906169
See Also: 906169
Attached patch remove_bar (obsolete) — Splinter Review
Assignee: nobody → mcastelluccio
Attachment #792982 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #792982 - Flags: review?(myk)
Attachment #793757 - Flags: review?(myk)
Comment on attachment 793757 [details] [diff] [review]
remove_bar

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

(In reply to Marco Castelluccio [:marco] from comment #0)
> After bug 776446 we have a small bar on the bottom of apps' windows. The
> menu separator in the popupset is causing this.
> We could just get rid of it for now and reintroduce it if/when we'll have a
> context menu with default entries.

That's ok, although I expect we'll have one soon enough.

In any case, note that the menuseparator should go *inside* the menupopup, so it should be possible to fix this bug by moving it there, although it didn't work in my quick test.

::: webapprt/content/webapp.js
@@ +242,5 @@
>  }
>  
>  nsContextMenu.prototype = {
>    initMenu: function(aXULMenu) {
> +    this.hasPageMenu = PageMenu.maybeBuildAndAttachMenu(document.popupNode, aXULMenu);

Nit: this change makes the line length exceed 80 columns; it should remain wrapped.
Attachment #793757 - Flags: review?(myk) → review+
Attached patch remove_barSplinter Review
Carrying r+.
Attachment #793757 - Attachment is obsolete: true
Attachment #794895 - Flags: review+
Keywords: checkin-needed
Backed out because one of the 4 webapps patches landed on your behalf was causing mochitest-other assertions. You really need to start running your patches through Try before requesting checkin. This is far from the first bustage we've had to backout for.
https://hg.mozilla.org/integration/fx-team/rev/3a3d9846ceca

https://tbpl.mozilla.org/php/getParsedLog.php?id=27007390&tree=Fx-Team
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #5)
> You really need to start running your patches through Try before requesting checkin.

I did: https://tbpl.mozilla.org/?tree=Try&rev=30a4e8eb935c

Maybe it's an intermittent failure.

By the way, this for sure is safe to land.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/796483a1459c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 26
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.