Closed
Bug 731832
Opened 11 years ago
Closed 11 years ago
Dropping HTMLImageElement.x broke this web page
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: admin, Assigned: Ms2ger)
References
Details
(Keywords: dev-doc-complete, regression)
Attachments
(3 files)
8.07 KB,
text/javascript
|
Details | |
3.23 KB,
patch
|
bzbarsky
:
review+
akeybl
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
3.49 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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....
Comment 1•11 years ago
|
||
Works for me with Firefox 6.0 Doesn't work starting with 7.0 So must have regressed in between.
Comment 2•11 years ago
|
||
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
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
![]() |
||
Comment 3•11 years ago
|
||
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
![]() |
||
Updated•11 years ago
|
Component: Layout → DOM: Core & HTML
QA Contact: layout → general
Assignee | ||
Comment 5•11 years ago
|
||
Because it's using .x/.y inside an if (!document.all && document.getElementById) block
![]() |
||
Comment 6•11 years ago
|
||
Hmm. What does it do on the other side of that block?
Assignee | ||
Comment 7•11 years ago
|
||
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;
![]() |
||
Comment 8•11 years ago
|
||
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•11 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.
![]() |
||
Comment 10•11 years ago
|
||
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?
Comment 11•11 years ago
|
||
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
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → Ms2ger
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #617963 -
Flags: review?(bzbarsky)
![]() |
||
Comment 13•11 years ago
|
||
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•11 years ago
|
||
Attachment #617987 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 15•11 years ago
|
||
As usual, this will break c-c.
Assignee | ||
Updated•11 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 16•11 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•11 years ago
|
||
Try pushes: https://tbpl.mozilla.org/?tree=Try&rev=3195ccb1b0bc (aurora) https://tbpl.mozilla.org/?tree=Try&rev=8de48e0a39c1 (m-c)
![]() |
||
Comment 18•11 years ago
|
||
Comment on attachment 617987 [details] [diff] [review] Trunk backout r=me
Attachment #617987 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 19•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/75c7378c87b6
Comment 20•11 years ago
|
||
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•11 years ago
|
Keywords: checkin-needed
Whiteboard: [c-n attachment 617963 for aurora]
Comment 21•11 years ago
|
||
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?
Updated•11 years ago
|
Keywords: checkin-needed
Whiteboard: [c-n attachment 617963 for aurora]
Updated•11 years ago
|
Summary: Issues loading a specific web page → Dropping HTMLImageElement.x broke this web page
Comment 22•11 years ago
|
||
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•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/30d0f3aa2836
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → mozilla15
Comment 24•11 years ago
|
||
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•11 years ago
|
||
Should be Firefox 14, right?
Comment 26•11 years ago
|
||
Is it? I was told Aurora 13. I can fix it if that's wrong. Can anyone advise?
Assignee | ||
Comment 27•11 years ago
|
||
Flags say "status-firefox13: affected "
Comment 28•11 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.
![]() |
||
Comment 29•11 years ago
|
||
Hmm. Maybe this should ask for beta approval, then. It certainly seems like this was approved for 13 initially.
Comment 30•11 years ago
|
||
Keyword should be dev-doc-needed or the doc changed: https://developer.mozilla.org/en/DOM/HTMLImageElement#Other_properties_%28Gecko%29
![]() |
||
Comment 31•11 years ago
|
||
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?
Comment 32•11 years ago
|
||
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•11 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.
![]() |
||
Comment 34•11 years ago
|
||
Hmm. Then why did the approval comment talk about Firefox 13? Anyway....
Comment 35•11 years ago
|
||
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-
Comment 36•11 years ago
|
||
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.
Description
•