Tools > Social menu not showing up on nightly since bug 772808 was fixed.

RESOLVED FIXED in Firefox 19

Status

defect
RESOLVED FIXED
7 years ago
4 months ago

People

(Reporter: markh, Assigned: markh)

Tracking

unspecified
Firefox 19
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox17 unaffected, firefox18 unaffected)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

7 years ago
The menu is not showing up on nightly since bug 772808 was fixed.  This is a different problem than bug 804242 where it sometimes isn't showing up on Aurora/Beta.
Assignee

Updated

7 years ago
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee

Comment 1

7 years ago
Assignee: nobody → mhammond
Attachment #674088 - Flags: review?(mixedpuppy)
Attachment #674088 - Flags: review?(jaws)
Assignee

Comment 2

7 years ago
Comment on attachment 674088 [details] [diff] [review]
update command instead of individual items

This patch doesn't work correctly - see bug 772808 comment 12.
Attachment #674088 - Flags: review?(mixedpuppy)
Attachment #674088 - Flags: review?(jaws)
Assignee

Comment 3

7 years ago
Setting .hidden doesn't work correctly, and neither does removing the hidden attribute (bug 772808 only updates the hidden attribute if the command has the attribute).  Fortunately XUL elements are hidden explicitly by checking hidden="true" - so hidden="false" works.
Attachment #674088 - Attachment is obsolete: true
Attachment #674154 - Flags: review?(mixedpuppy)
Attachment #674154 - Flags: review?(jaws)
Attachment #674154 - Flags: review+
Attachment #674154 - Flags: review?(mixedpuppy)
Attachment #674154 - Flags: review?(jaws)
(In reply to Mark Hammond (:markh) from comment #3)
> Created attachment 674154 [details] [diff] [review]
> Use setAttribute instead of .hidden
> 
> Setting .hidden doesn't work correctly, and neither does removing the hidden
> attribute (bug 772808 only updates the hidden attribute if the command has
> the attribute).

Setting .hidden and removing the attribute should be equivalent (the hidden setter removes the attribute under the hood). That it doesn't work is a bug in the patch for bug 772808, right? Is there a bug on file?
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #5)
> That it doesn't work is a bug in the patch for bug 772808, right?

Attribute removals were generally not observed prior to and regardless of bug 772808, afaik.
Assignee

Comment 7

7 years ago
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #5)

> Setting .hidden and removing the attribute should be equivalent (the hidden
> setter removes the attribute under the hood).

Right - IIRC that part was working.

> That it doesn't work is a bug
> in the patch for bug 772808, right? Is there a bug on file?

772808 explicitly only copies the value of the "hidden" attribute from the command to the menu item if the command has a "hidden" attribute.  The fact that setting .hidden=false removes the attribute means that whatever value the menuitem had for hidden stays the same - so it isn't also removed from the menuitem (ie, the menuitem remains hidden even if the command had the attribute removed).

The code touched by bug 772808 uses this same behaviour for the 'label', 'accesskey', 'checked' and now 'hidden' attributes.  However, the attribute 'disabled' is treated differently - if no such attribute exists on the command it is also removed from the menuitem.  At a guess, I'd say the bug here is that 772808 should have treated 'hidden' in the same way it treats 'disabled', but I'm not familiar enough with things to know for sure - and given that XUL's css explicitly uses [hidden="true"] rather than simply [hidden], it isn't clear to me that this really is the/a problem.  So there is no bug on file I'm aware of.
https://hg.mozilla.org/mozilla-central/rev/5077791b4ea4
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
(In reply to Mark Hammond (:markh) from comment #7)
> At a guess, I'd say the bug here is that 772808 should have treated 'hidden' in the
> same way it treats 'disabled', but I'm not familiar enough with things to know for
> sure

Yes, I think that's exactly right.
(In reply to Dão Gottwald [:dao] from comment #6)
> Attribute removals were generally not observed prior to and regardless of
> bug 772808, afaik.

The XUL properties for these attributes remove the attribute, so not observing attribute removal is a pretty obvious bug. What other cases are there that aren't currently covered by bug 805653?
I can't remember where. I'm sure this isn't the first time we're setting attributes to false rather than removing them, but it's possible that I confused attribute observing with attribute persisting.

Updated

4 months ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.