Back/Forward button not showing up in FF. DOM error?

RESOLVED FIXED in Firefox 3.7a4

Status

()

Firefox
Build Config
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: Jeremy Huddleston, Assigned: Hanspeter Niederstrasser)

Tracking

Trunk
Firefox 3.7a4
All
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

8 years ago
This was reported in bug #527612 but is not related to that issue:

application does not have all the chrome properly installed, as shown in the
attached image.

DOM inspector for the OBJDIR version, shows the following for the forward
button:

list-style-image:
url("moz-icon://stock/gtk-go-forward-ltr?size=toolbar&state=disabled");

while that computed style entry does not exist when running from INSTALLDIR
(although the forward-button (and other missing icons) do show up as nodes in
DOM Inspector).  I think this is a different bug, but I don't have any time
points that differentiate between this and comment #0.  I will open a new bug
if this is a different issue.
What do you mean by "not showing up"? Do you mean the icon on the button, or the button itself? It's not clear to me why you're bringing up the objdir vs. installdir difference, either.
Component: DOM → General
Product: Core → Firefox
QA Contact: general → general
Version: 1.9.2 Branch → Trunk
(Assignee)

Comment 2

8 years ago
Gavin,

I was the commenter that brought up that issue in the other bug.  I brought up objdir vs installdir because firefox had all chrome available when running from objdir/dist/bin, but not when running from installdir/bin.  It's been a while since I looked at this specific issue, but if I remember correctly, it was the icons on the buttons that were not showing up, but the buttons could still be seen on hover/click, but they were very narrow (about 10-20% the normal width).

I tracked down the issue to package-manifest looking for libimgicon.so, but it was being built as libimgicon.dylib (Darwin/X11) ...so it wasn't getting installed and so most chrome icons weren't being loaded when firefox was run from installdir/bin.  I patched package-manifest.in to look for libimgicon.dylib and that made chrome look correct when running firefox from installdir.
(Reporter)

Comment 3

8 years ago
Created attachment 429857 [details] [diff] [review]
untested possible fix

This is likely the fix then.  I'll test it before requesting a review.
(Reporter)

Updated

8 years ago
Attachment #429857 - Flags: review?(benjamin)
(Reporter)

Comment 4

8 years ago
Comment on attachment 429857 [details] [diff] [review]
untested possible fix

Tested fix
(Reporter)

Updated

8 years ago
Assignee: nobody → benjamin
(Reporter)

Updated

8 years ago
Assignee: benjamin → nobody
Component: General → Build Config
QA Contact: general → build.config
Hardware: PowerPC → All
Version: Trunk → 3.6 Branch
(Assignee)

Comment 5

8 years ago
Created attachment 430025 [details] [diff] [review]
patch against mozilla-central
[Checkin: Comment 6]

Jeremy, that's a much cleaner fix than my directly changing .so to .dylib in package-manifest.  I updated your patch to 1) reflect slight differences between mozilla-central and 1.9.2, and 2) removed the '.' between FILENAME and @DLL_SUFFIX@ since that macro already adds a period.
Attachment #430025 - Flags: review?

Updated

8 years ago
Attachment #430025 - Flags: review? → review+
(Reporter)

Updated

8 years ago
Attachment #429857 - Attachment is obsolete: true
Attachment #429857 - Flags: review?(benjamin)
(Reporter)

Updated

8 years ago
Keywords: checkin-needed

Updated

8 years ago
Assignee: nobody → niederstrasser
http://hg.mozilla.org/mozilla-central/rev/2b8a0778faf0
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a4
I actually just wrote a patch and filed bug 553635 today, which will get rid of this library (link it into libxul).
Target Milestone: Firefox 3.7a4 → ---
Flags: in-testsuite-
Target Milestone: --- → Firefox 3.7a4
Version: 3.6 Branch → Trunk
Created attachment 433955 [details] [diff] [review]
(Bv1-CC) Copy it to SeaMonkey
Attachment #433955 - Flags: review?(bugspam.Callek)
Created attachment 434249 [details] [diff] [review]
(Bv1a-CC) Copy it to SeaMonkey, Unify other cases

Bv1-CC, with additional nits.
Attachment #433955 - Attachment is obsolete: true
Attachment #434249 - Flags: review?(bugspam.Callek)
Attachment #433955 - Flags: review?(bugspam.Callek)
Comment on attachment 434249 [details] [diff] [review]
(Bv1a-CC) Copy it to SeaMonkey, Unify other cases

technically good to go; but over to KaiRo as I'm not sure if the preprocessing on the OS-Specific files is needed/warranted in most of these cases.
Attachment #434249 - Flags: review?(kairo)
Attachment #434249 - Flags: review?(bugspam.Callek)
Attachment #434249 - Flags: review+

Comment 11

8 years ago
Comment on attachment 434249 [details] [diff] [review]
(Bv1a-CC) Copy it to SeaMonkey, Unify other cases

>-@BINPATH@/components/libalerts_s.dylib
>+@BINPATH@/components@DLL_PREFIX@alerts_s@DLL_SUFFIX@

1) This hunk is wrong, it misses a / character.

2) I see no reason for making single-platform lines harder to read.

3) I don't see any real change in there, nothing you did actually "copy".
Attachment #434249 - Flags: review?(kairo) → review-
Attachment #430025 - Attachment description: patch against mozilla-central → patch against mozilla-central [Checkin: Comment 6]
Created attachment 435392 [details] [diff] [review]
(Bv2-CC) Copy it to SeaMonkey, Unify other cases
[Moved to bug 717702]

Bv1a-CC, with comment 11 suggestion(s).

(In reply to comment #11)
> 1) This hunk is wrong, it misses a / character.

Good catch: fixed.

> 2) I see no reason for making single-platform lines harder to read.

See bug 554017 comment 2...

> 3) I don't see any real change in there, nothing you did actually "copy".

I copied this bug and bug 554017.
Attachment #434249 - Attachment is obsolete: true
Attachment #435392 - Flags: review?(kairo)

Comment 13

8 years ago
Comment on attachment 435392 [details] [diff] [review]
(Bv2-CC) Copy it to SeaMonkey, Unify other cases
[Moved to bug 717702]

(In reply to comment #12)
> > 2) I see no reason for making single-platform lines harder to read.
> 
> See bug 554017 comment 2...

Things like that deserve their own bug with a fitting description that describes that you're changing how we do our stuff in those files.

> > 3) I don't see any real change in there, nothing you did actually "copy".
> 
> I copied this bug and bug 554017.

I still don't see anything in this patch that actually deals with the bug it's attached to.
Attachment #435392 - Flags: review?(kairo) → review-
Comment on attachment 435392 [details] [diff] [review]
(Bv2-CC) Copy it to SeaMonkey, Unify other cases
[Moved to bug 717702]

(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #13)

> Things like that deserve their own bug with a fitting description that
> describes that you're changing how we do our stuff in those files.

Bug 717702, at last.

> I still don't see anything in this patch that actually deals with the bug
> it's attached to.

'nullplugin' and 'imgicon' lines were copied++ from this bug.
There are gone in the meantime anyway.
Attachment #435392 - Attachment description: (Bv2-CC) Copy it to SeaMonkey, Unify other cases → (Bv2-CC) Copy it to SeaMonkey, Unify other cases [Moved to bug 717702]
Attachment #435392 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.