Dropping HTMLImageElement.x broke this web page

RESOLVED FIXED in Firefox 14

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Franklin S Werren, Assigned: Ms2ger)

Tracking

({dev-doc-complete, regression})

Trunk
mozilla15
dev-doc-complete, regression
Points:
---

Firefox Tracking Flags

(firefox11 wontfix, firefox12- wontfix, firefox13- affected, firefox14- fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:10.0.2) Gecko/20100101 Firefox/10.0.2
Build ID: 20120216115113

Steps to reproduce:

Loaded up www.crownhill.com and the java buttons will not load right. Also shows up in Sea Monkey. And this issue is not lock to a specific OS


Actual results:

Loaded up www.crownhill.com and the java buttons will not load right. Also shows up in Sea Monkey.


Expected results:

Should load up with the drop down menus not showing up in the left side. Works fine in Opera and Google Chrome as well as Internet Explorer in windows. I use Ubuntu myself. Complaint came in via customer and owner of www.crownhill.com. I am a bit fickle how to fix problem on my end....
Works for me with Firefox 6.0
Doesn't work starting with 7.0

So must have regressed in between.
Keywords: regressionwindow-wanted
OS: Linux → All
Hardware: x86_64 → All
Version: 10 Branch → Trunk
Last good nightly: 2011-05-28
First bad nightly: 2011-05-29

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1f25c65e9335&tochange=5d7b5f2ea603
Component: Untriaged → Layout
Keywords: regressionwindow-wanted
Product: Firefox → Core
QA Contact: untriaged → layout
Status: UNCONFIRMED → NEW
Ever confirmed: true
hg bisect says:

The first bad revision is:
changeset:   70271:f13a2020b5b9
user:        Ms2ger <ms2ger@gmail.com>
date:        Sat May 28 09:43:43 2011 +0200
summary:     Bug 587021 - drop HTMLImageElement.x/.y; r=sicking sr=jst

That bug report said these are not supported in IE.  Does this site work in IE, and if so, why?
Blocks: 587021
Keywords: regression
Ah, comment 0 says it does work in IE.  Again, the question is why.
Component: Layout → DOM: Core & HTML
QA Contact: layout → general
(Assignee)

Comment 5

5 years ago
Because it's using .x/.y inside an if (!document.all && document.getElementById) block
Hmm.  What does it do on the other side of that block?
(Assignee)

Comment 7

5 years ago
Created attachment 606861 [details]
/buttons_1/xaramenu.js


    x = event.clientX - event.offsetX + document.body.scrollLeft - document.body.clientLeft;
    y = event.clientY - event.offsetY + document.body.scrollTop - document.body.clientTop - bd;
    dx = event.srcElement.offsetWidth;
    dy = event.srcElement.offsetHeight;
OK, so that code wouldn't work in Gecko even if the site changed the sniffing because we don't support offsetX, yes?

Quite honestly, I think we should consider backing out bug 587021 and then specifying .x and .y on images, since everyone but IE implements it.

Comment 9

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #8)
> OK, so that code wouldn't work in Gecko even if the site changed the
> sniffing because we don't support offsetX, yes?
> 
> Quite honestly, I think we should consider backing out bug 587021 and then
> specifying .x and .y on images, since everyone but IE implements it.

I'm in agreement on backing out bug 587021.
Nominating for tracking; I have no idea why I didn't do this back when I made comment 8.

Again, I think the spec here just needs to get changed and we should back out the removal of the properties...  Jonas, thoughts?
tracking-firefox12: --- → ?
tracking-firefox13: --- → ?
tracking-firefox14: --- → ?
I don't think this actually needs to track for a specific release since it's a regression from FF7. That being said, we'd definitely approve a backout for FF13 if nominated.
status-firefox11: --- → wontfix
status-firefox12: --- → wontfix
status-firefox13: --- → affected
status-firefox14: --- → affected
tracking-firefox12: ? → -
tracking-firefox13: ? → -
tracking-firefox14: ? → -
(Assignee)

Updated

5 years ago
(Assignee)

Updated

5 years ago
Assignee: nobody → Ms2ger
(Assignee)

Comment 12

5 years ago
Created attachment 617963 [details] [diff] [review]
Aurora backout
Attachment #617963 - Flags: review?(bzbarsky)
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

That should be nsIntPoint not nsPoint, right?  Yes, I know it used to be wrong before, but while we're here....

r=me
Attachment #617963 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 14

5 years ago
Created attachment 617987 [details] [diff] [review]
Trunk backout
Attachment #617987 - Flags: review?(bzbarsky)
(Assignee)

Comment 15

5 years ago
As usual, this will break c-c.
(Assignee)

Updated

5 years ago
Keywords: dev-doc-needed
(Assignee)

Comment 16

5 years ago
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: The menu on this particular site doesn't work for another 6 weeks
Testing completed (on m-c, etc.): Not yet
Risk to taking this patch (and alternatives if risky): small; adds back two attributes to an interface (+ uuid change)
String changes made by this patch: None
Attachment #617963 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 17

5 years ago
Try pushes:
https://tbpl.mozilla.org/?tree=Try&rev=3195ccb1b0bc (aurora)
https://tbpl.mozilla.org/?tree=Try&rev=8de48e0a39c1 (m-c)
Comment on attachment 617987 [details] [diff] [review]
Trunk backout

r=me
Attachment #617987 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 19

5 years ago
https://hg.mozilla.org/mozilla-central/rev/75c7378c87b6
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

[Triage comment]
Please go ahead and do the backout on Aurora (Firefox 13).
Attachment #617963 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Whiteboard: [c-n attachment 617963 for aurora]
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

Hold off on landing this for a bit - I'd like to make doubly sure a UUID change won't cause any weird compatibility issues. Putting back to a?
Attachment #617963 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora?
Keywords: checkin-needed
Whiteboard: [c-n attachment 617963 for aurora]

Updated

5 years ago
Summary: Issues loading a specific web page → Dropping HTMLImageElement.x broke this web page
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

[Triage Comment]
Spoke with Gavin last week. He let me know there was a very low chance that this would cause binary incompatibility issues. We'll be on the lookout.
Attachment #617963 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 23

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/30d0f3aa2836
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox14: affected → fixed
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla15
Docs updated:

https://developer.mozilla.org/en/DOM/HTMLImageElement#Other_properties_%28Gecko%29

Mentioned on Firefox 13 for developers.
Keywords: dev-doc-needed → dev-doc-complete

Comment 25

5 years ago
Should be Firefox 14, right?
Is it? I was told Aurora 13. I can fix it if that's wrong. Can anyone advise?
(Assignee)

Comment 27

5 years ago
Flags say "status-firefox13: 	affected "

Comment 28

5 years ago
Here's the history:

2012-04-16: Comment 11 "we'd definitely approve a backout for FF13"
2012-04-24: Comment 12: Patch created for Aurora.
2012-04-25: Aurora bumped to FF14.
2012-05-05: Comment 23: Landed on Aurora.
Hmm.  Maybe this should ask for beta approval, then.  It certainly seems like this was approved for 13 initially.

Comment 30

5 years ago
Keyword should be dev-doc-needed or the doc changed:
https://developer.mozilla.org/en/DOM/HTMLImageElement#Other_properties_%28Gecko%29
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

Asking for beta approval, per above.

Note that this was approved for aurora initially before aurora went to beta, but didn't land until _after_ aurora went to beta.

I can totally understand if beta approval is denied to limit risk...  Note that there is an interface change involved.
Attachment #617963 - Flags: approval-mozilla-beta?
Dev-doc-needed set so that we can update the doc once approval for beta is decided.
Keywords: dev-doc-complete → dev-doc-needed
(Assignee)

Comment 33

5 years ago
(In reply to Boris Zbarsky (:bz) from comment #31)
> Note that this was approved for aurora initially before aurora went to beta,
> but didn't land until _after_ aurora went to beta.

AFAICT, it was approved on 2012-04-25, and the merge was on 2012-04-24.
Hmm.  Then why did the approval comment talk about Firefox 13?  Anyway....
Comment on attachment 617963 [details] [diff] [review]
Aurora backout

No dupes suggest low user effect, we'll have to let this ride the trains.
Attachment #617963 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Docs revised to reflect this change was implemented in Firefox 14, not 13.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.