Closed
Bug 332246
Opened 18 years ago
Closed 17 years ago
scrollIntoView(false) doesn't work correctly for inline elements that wrap at multiple lines
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: martijn.martijn, Assigned: martijn.martijn)
References
(Depends on 1 open bug)
Details
(Keywords: testcase)
Attachments
(4 files, 1 obsolete file)
1.98 KB,
text/html
|
Details | |
2.01 KB,
text/html
|
Details | |
940 bytes,
patch
|
roc
:
review+
roc
:
superreview+
roc
:
approval1.9+
|
Details | Diff | Splinter Review |
3.34 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
See upcoming testcase. Clicking on the 'Show bottom' button doesn't work correctly in Mozilla. It only scrolls the first line into view (from the bottom). It should show the last line into view from the bottom, like IE6 is doing.
Assignee | ||
Comment 1•18 years ago
|
||
Assignee | ||
Comment 2•18 years ago
|
||
This makes it work for me.
Attachment #216762 -
Flags: review?(jst)
Comment 3•18 years ago
|
||
Comment on attachment 216762 [details] [diff] [review] patch Looks good to me, but bz or dbaron should sr.
Attachment #216762 -
Flags: superreview?(dbaron)
Attachment #216762 -
Flags: review?(jst)
Attachment #216762 -
Flags: review+
Would a fix for bug 66619 also fix this?
Assignee | ||
Comment 5•18 years ago
|
||
(In reply to comment #4) > Would a fix for bug 66619 also fix this? Yes, I think so. Not sure what the patch in that bug is also trying to fix, though.
Depends on: 66619
Assignee | ||
Updated•18 years ago
|
Attachment #216762 -
Flags: superreview?(dbaron)
Assignee | ||
Comment 6•17 years ago
|
||
Assignee | ||
Comment 7•17 years ago
|
||
The first value in testcase2 returns 101 in current trunk builds, that shouldn't happen, so I filed bug 401904 for it.
Assignee | ||
Comment 8•17 years ago
|
||
This is now enough to make it work correctly.
Attachment #216762 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Assignee: general → martijn.martijn
Assignee | ||
Comment 9•17 years ago
|
||
Attachment #286852 -
Flags: review?(dbaron)
Assignee | ||
Updated•17 years ago
|
Attachment #286845 -
Flags: review?(dbaron)
Comment 10•17 years ago
|
||
I see this happening on Mac OS X 10.4 as well. To me it does not matter if it is an inline or block element or whether the element text wraps or not. I can reproduce the bug with any element.
Flags: blocking1.9?
OS: Windows XP → All
Hardware: PC → All
Comment 11•17 years ago
|
||
This doesn't appear to be a regression from 1.8 - but would take the patch - so marking - for blocking but if you get reviews on patch ask for patch approval
Flags: blocking1.9? → blocking1.9-
Assignee | ||
Comment 12•17 years ago
|
||
Comment on attachment 286845 [details] [diff] [review] patch Robert, perhaps you could review?
Attachment #286845 -
Flags: review?(dbaron) → review?(roc)
Assignee | ||
Updated•17 years ago
|
Attachment #286852 -
Flags: review?(dbaron) → review?(roc)
Attachment #286845 -
Flags: superreview+
Attachment #286845 -
Flags: review?(roc)
Attachment #286845 -
Flags: review+
Attachment #286845 -
Flags: approval1.9+
Assignee | ||
Comment 13•17 years ago
|
||
Robert, you forgot to review (or take a quick look at) the mochitest.
Comment on attachment 286852 [details] [diff] [review] mochitest I have very little enthusiasm for reviewing tests :-)
Attachment #286852 -
Flags: review?(roc) → review+
Assignee | ||
Comment 15•17 years ago
|
||
Checking in nsGenericHTMLElement.cpp; /cvsroot/mozilla/content/html/content/src/nsGenericHTMLElement.cpp,v <-- nsGen ericHTMLElement.cpp new revision: 1.745; previous revision: 1.744 done Checked into trunk. Checking in Makefile.in; /cvsroot/mozilla/content/html/content/test/Makefile.in,v <-- Makefile.in new revision: 1.35; previous revision: 1.34 done RCS file: /cvsroot/mozilla/content/html/content/test/test_bug332246.html,v done Checking in test_bug332246.html; /cvsroot/mozilla/content/html/content/test/test_bug332246.html,v <-- test_bug3 32246.html initial revision: 1.1 done Mochitest checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee | ||
Comment 16•17 years ago
|
||
MacOSX Darwin 8.8.4 qm-xserve01 dep unit test was failing with the mochitest, so I removed this test from the Makefile.in file for now.
Flags: in-testsuite+ → in-testsuite?
Assignee | ||
Comment 17•17 years ago
|
||
Apparently, it also fails on Linux qm-centos5-01 dep unit test.
Assignee | ||
Comment 18•17 years ago
|
||
Ok, it turns out that Linux qm-centos5-01 dep unit test is failing because it doesn't suffer from bug 401904, so it's failing on the TODO item! Mac is failing for the first 2 tests, it returns 102 for both of them. I filed bug 411297 for that. So I guess I could alter the Mochitest to do platform specific test to take all of that into account.
Assignee | ||
Comment 19•17 years ago
|
||
Ok, I added some platform specific checks now in the mochitest, like this: //bug 401904 for windows, bug 411297 for the Mac if (navigator.platform == "Win32" || navigator.platform.indexOf("Mac") == 0) todo(a1.scrollTop == 100, "Wrong scrollTop value!"); else ok(a1.scrollTop == 100, "Wrong scrollTop value!"); a2.scrollIntoView(false); if (navigator.platform.indexOf("Mac") == 0) //Mac bug 411297 again todo((a1.scrollHeight - a1.offsetHeight - a1.scrollTop) == 100, "Wrong (scrollHeight - offsetHeight - scrollTop) value!"); else ok((a1.scrollHeight - a1.offsetHeight - a1.scrollTop) == 100, "Wrong (scrollHeight - offsetHeight - scrollTop) value!"); It's ugly, but it should work. The platform specific checks can be removed as soon as the corresponding bugs would be removed.
Flags: in-testsuite? → in-testsuite+
Comment 20•17 years ago
|
||
Since this checkin, on my nightly CVS trunk builds, forums such as the example below now jump to the top of the page when clicking the Prev or Next links with the page scrolled down. In previous nightlies, the scoll position remained where it was when clicking the links. This is a major nuisance, although I didn't look at the JavaScript being run to see if it was working correctly or incorrectly before. http://forums.roadbikereview.com/showthread.php?t=116961
Assignee | ||
Comment 21•17 years ago
|
||
Brad, thanks for mentioning this, I filed bug 411665 for it. Note that IE and Opera do the same, so I actually think that Mozilla is doing the correct thing now (although it looks ugly). But maybe the ugly flash of scrolling can be reduced in some other way. The most simple thing off course would be if the site in question would fix their broken script.
You need to log in
before you can comment on or make changes to this bug.
Description
•