Closed Bug 585877 Opened 14 years ago Closed 14 years ago

remove document.height / document.width

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: annevk, Assigned: Ms2ger)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 2 obsolete files)

It would be great if support for these proprietary features could be removed from Gecko. Opera encountered one problem so far with not supporting document.width and since Internet Explorer does not have them either it may not be too late to not forever have to support this.
Assignee: nobody → Ms2ger
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Attached patch Patch v1 (deleted) — Splinter Review
Attachment #464390 - Flags: review?(jonas)
Did you check that this passes tryserver?
I don't have try access. Tests pass locally, though. Does this need sr?
Attachment #464390 - Flags: superreview?(jst) → superreview+
Keywords: dev-doc-needed
approval2.0?
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Blocks: 587629
blocking2.0: --- → beta4+
This change broke graphs.mozilla.org. A cursory google code search shows that document.height is far from unused, though mostly on aging code paths. I don't see a good reason to take this for Firefox 4. I'm going to back this out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
"Opera had no problems" isn't sufficient research on which basis to remove things, especially without doing a basic search for use. Please do back it out, Sayre, and let's be a little more careful in the future?
Attachment #464390 - Flags: approval2.0+ → approval2.0-
Note that "IE has never supported them" was the stronger reason that we felt ok with this. I'm fine with not taking this for firefox 4, however I don't think "graphserver uses it" is a very good reason for keeping a feature in the web platform. Will look more at google code results though.
Ms2ger: Would you mind writing up a patch that adds warnings to the console whenever these properties are used? Probably want to add a flag so that we only warn once per document or some such.
This is not worth focusing on for Firefox 4, imho.
Please back out of mozilla-central default and GECKO20b4_20100817_RELBRANCH
(removing blocking:beta4+ since the issue's no longer in that beta)
blocking2.0: beta4+ → ---
The MDC changes also need to be reverted.
Anne's wrong-footing Sicking! What next? Some Googler bamboozling a JS purist into removing __proto__? :-/ /be
The documentation edits need to backed out as well.
Bredan, geez. Jonas expressed interest in removing legacy stuff on the public-html list so I found some and reported a bug with the information I had on Opera and IE. Did I know your internal systems rely on it...
Anne, I kid (mostly :-/). But it is not just our internal systems that use this old stuff. /be
Brendan: It almost is only us using it. Google code search turned up a lot of things that didn't use document_height or other similarly named things, a lot of stuff commented out, and a lot of stuff that falls back to using other properties, or that also depends on layers and thus is already broken. Here are a few things that might be breaking: http://nk-gesture.googlecode.com/svn (nkGestures.js) http://accessext.googlecode.com/svn (treebox_class.js) Some gecko specific thing, also uses XUL git://github.com/cauld/sideline.git (sideline.js) http://konverta4.googlecode.com/svn (functions.js) http://hyperic-hq.svn.sourceforge.net/svnroot/hyperic-hq (footer.js) git://github.com/cauld/sideline.git (ti-sideline-synch-hack.js)
(In reply to comment #21) > > Here are a few things that might be breaking: Removing these is not a terrible idea, but it will take time to research. The fact that one of our sites broke immediately is not a dealbreaker, but it is a bad sign. Thinking more, I can't see how this change improves the Web very much if there's no risk in removing these properties. I'm pretty sure there are much more important things to focus on.
Before removing properties like document.height/.width we could warn in the error console about use of deprecated feature. That approach has worked reasonable well when removing/no-op'ing some Netscapeisms from the event handling code.
Attached patch Warning patch v1 (obsolete) (deleted) — Splinter Review
(This doesn't apply to trunk yet, due to Mounir's recent changes.)
Attachment #467844 - Flags: review?(jonas)
> Created attachment 467844 [details] [diff] [review] > Warning patch v1 Wouldn't it be better to recommend document.body.clientHeight/clientWidth ? It has better backwards and browser compatibility than document.body.getBoundingClientRect()
Comment on attachment 467844 [details] [diff] [review] Warning patch v1 Though I would also recommend clientWidth/clientHeight.
Attachment #467844 - Flags: review?(jonas) → review+
Comment on attachment 467844 [details] [diff] [review] Warning patch v1 Okay, will fix.
Attachment #467844 - Flags: approval2.0?
Comment on attachment 467844 [details] [diff] [review] Warning patch v1 Please re-request approval once a patch that's ready to land is available.
Attachment #467844 - Flags: approval2.0? → approval2.0-
Attached patch Warning patch v2 (obsolete) (deleted) — Splinter Review
Attachment #467844 - Attachment is obsolete: true
Attachment #468978 - Flags: approval2.0?
Attachment #468978 - Flags: approval2.0? → approval2.0+
Attached patch Warning patch (checked in) (deleted) — Splinter Review
Thanks.
Attachment #468978 - Attachment is obsolete: true
Keywords: checkin-needed
Whiteboard: [c-n:"Warning patch for checkin"]
Target Milestone: mozilla2.0b4 → Future
Keywords: checkin-needed
Whiteboard: [c-n:"Warning patch for checkin"]
Attachment #469385 - Attachment description: Warning patch for checkin → Warning patch (checked in)
Status: REOPENED → ASSIGNED
Whiteboard: [needs landing]
Depends on: post2.0
Blocks: 53076
Does this really need landing? Looks like it landed. Should it be resolved FIXED?
Whiteboard: [needs landing] → [needs landing][approved-patches-landed]
Yes, the main patch here still needs to land, after we branch. It was backed out: http://hg.mozilla.org/mozilla-central/rev/9aa39b619a19
Whiteboard: [needs landing][approved-patches-landed] → [needs landing][approved-patches-landed][not-ready-for-cedar]
Whiteboard: [needs landing][approved-patches-landed][not-ready-for-cedar] → [need gk2.2 ship][not-ready-for-cedar]
No longer depends on: post2.0
Whiteboard: [need gk2.2 ship][not-ready-for-cedar] → [need gk2.2 ship]
Whiteboard: [need gk2.2 ship] → [need gk2.2 ship][not-ready-for-cedar]
I was going to land this, but I wasn't sure whether the warning patch should be backed out or not (I think it should).
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: [need gk2.2 ship][not-ready-for-cedar]
Target Milestone: Future → mozilla6
Blocks: 634755
Blocks: 651651
Blocks: 651750
(In reply to comment #25) > Wouldn't it be better to recommend > document.body.clientHeight/clientWidth ? > > It has better backwards and browser compatibility than > document.body.getBoundingClientRect() In the (admittedly unusual) case where the body element has a border, then clientHeight/clientWidth deliberately excludes that. (Padding is not excluded.) On the other hand, getBoundingClientRect.height/width returns a float...
Depends on: 653233
Depends on: 653292
Depends on: 680301
Depends on: 692616
Blocks: 698876
Depends on: 714577
Why was this functionality removed? This feature is used in Sun / Oracle ILOM web management interfaces and removal has broken the ability to manage systems using Firefox 6 web browsers and above. Users are then forced to use other browsers. Although Firefox 6 and above support has been fixed in newer ILOM firmware versions by Oracle on some systems, it still means that older historical systems will not work with Firefox 6+. Surely it would have been better to have an about:config setting to re-enable this functionality if needed?
Depends on: 694931
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: