Closed Bug 392671 Opened 12 years ago Closed 12 years ago

google reader jumpy when tagging

Categories

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

defect
Not set
major

Tracking

()

RESOLVED FIXED

People

(Reporter: cannedbrain+mozilla, Assigned: roc)

References

()

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a8pre) Gecko/2007081701 Camino/2.0a1pre (like Firefox/3.0a8pre)
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.9a8pre) Gecko/2007081701 Camino/2.0a1pre (like Firefox/3.0a8pre)

When adding a tag to an article in google reader, the page would jump one-two pages down and auto complete does not work. It doesn't happen in 1.6pre1.

Reproducible: Always

Steps to Reproduce:
1. click "Add tags"
2. start typing
Actual Results:  
The page would automagically scroll down and it won't autocomplete as you start typing.

Expected Results:  
Should stay where it it & display matching tags as you type.

Here's the errors found in Console:
https://dav.earthlink.net/cannedbrain/Public/jserrors.txt
The fact that you're getting some CSS that appears to be intended for Safari suggests to me that either Google has a bug on their end that's somehow sending bad CSS to Camino, or you're spoofing your user-agent in some manner that makes Google think you're using Safari.

Can you try it with a fresh profile and see if you still see the same problem?
(In reply to comment #2)
> The fact that you're getting some CSS that appears to be intended for Safari
> suggests to me that either Google has a bug on their end that's somehow sending
> bad CSS to Camino, or you're spoofing your user-agent in some manner that makes
> Google think you're using Safari.

Lots of sites use vendor-specific tags in CSS they send to everyone, since browsers will ignore CSS they don't understand, by spec.
All those entries about css 'errors' are just warnings (and most refer to Win IE stuff).
The real question: does Google Reader work correctly with the latest Minefield build(s) ? Iirc there was a comment on the forums from someone with problems about it.
(In reply to comment #2)
> The fact that you're getting some CSS that appears to be intended for Safari
> suggests to me that either Google has a bug on their end that's somehow sending
> bad CSS to Camino, or you're spoofing your user-agent in some manner that makes
> Google think you're using Safari.
> 
> Can you try it with a fresh profile and see if you still see the same problem?
> 

I don't spoof my user-agent. It works fine with the latest branch, like I said. I tried it with a fresh profile and the problem still persisted. On a side note, the page appearance (the tabs on top of the page) is also messed up in the latest trunk:

http://cannedbrain.imap.cc/branch.png
http://cannedbrain.imap.cc/trunk.png
I can't repro this with trunk; adding tags works fine for me. Does anyone else see it?
(In reply to comment #6)

Yes I could reproduce it (testing on the BBC-world feed). 
STR
Scroll down the view list,
try adding a tag to one of the bottom items/articles.
result
The blue box with the input field is about 150px down from the selected item (which is scrolled out of view), and the auto-complete box some 200~300px lower.

I checked with Minefield just to be sure. It has the same issue as Camino.

The page appearance issue noted in comment 5 is a known issue with absolute positioning, floated tables (and is not Camino specific either): bug 370248.
This regressed between Minefield builds 20070530 Minefield/3.0a5pre and 20070531 Minefield/3.0a5pre

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-05-30+03%3A00%3A00&maxdate=2007-05-31+04%3A00%3A00&cvsroot=%2Fcvsroot

Possibly a regression from 174397 ?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Product: Camino → Core
QA Contact: general → general
Version: unspecified → Trunk
Yeah, this could very well be a regression somehow from bug 174397.

I found this code in the Google reader:
Nd=function(a){var b=Xc(a);if(!ka(ik)){ik=hr&&!(jr(ue,MB)>=0)}var c=new Na(0,0),d=Yg(b);if(a==d){return c}var e=null,f;if(a.getBoundingClientRect){f=a.getBoundingClientRect();var g=d.scrollTop,h=d.scrollLeft;c.x=f.left+h;c.y=f.top+g}else if(b.getBoxObjectFor&&!ik){f=b.getBoxObjectFor(a);var i=b.getBoxObjectFor(d);c.x=f.screenX-i.screenX;c.y=f.screenY-i.screenY}else{c.x=a.offsetLeft;c.y=a.offsetTop;e=a.offsetParent;
if(e!=a){while(e){c.x+=e.offsetLeft;c.y+=e.offsetTop;e=e.offsetParent}}if(hh||tf&&kk(a,NB)==Tl){c.y-=b.body.offsetTop}e=a.parentNode;while(e&&e!=d){c.x-=e.scrollLeft;if(!hh||e.tagName!=OB){c.y-=e.scrollTop}e=e.parentNode}}return c}

So in current trunk build the getBoundingClientRect path is followed in this case.
Not sure if there is a bug in Firefox here or with the website code. I suspect the latter, though.
Blocks: 174397
Flags: blocking1.9?
The good news is that it's not taking the getBoxObjectFor path!

Do we have any hope for figuring out what this code is doing and/or why it's failing?
Attached file testcase
Ok, apparently this is a bug in Mozilla.
Mozilla doesn't take scrolling into account at all for getBoundingClientRect. Probably the same problem with getClientRects().
Oh, so IE does take scrolling into account? I was under the impression that it did not.
(In reply to comment #12)
> Oh, so IE does take scrolling into account? I was under the impression that it
> did not.

Yes, IE takes scrolling into account. The values can even become negative because of that.

alright then, this is a super-blocker
Does it take scrolling of the viewport into account?
Yes (will attach a testcase later).
Attached file testcase2
Testcase sucks quite a bit, but it shows that the viewport scrolling is taken into account.
Attached patch fixSplinter Review
This should do it.

Can someone verify that it fixes Google Reader? I'd rather not set it up just for this?
Assignee: nobody → roc
Status: NEW → ASSIGNED
Attachment #278866 - Flags: superreview?(dbaron)
Attachment #278866 - Flags: review?(dbaron)
Yeah, that patch fixes the Google Reader issue for me.
Testing that patch on OS X: Google Reader works just fine now.
Could someone confirm if this also fixes the behaviour described
here: http://forums.mozillazine.org/viewtopic.php?t=579385 ?
Blocks: 396694
Component: General → DOM: Mozilla Extensions
OS: Mac OS X → All
Hardware: Macintosh → All
Summary: 2.0a1pre + google reader = jumpy when tagging → google reader jumpy when tagging
So this does mean that the values returned from this function don't match up with client coords (as in getElementFromPoint and event.client*) due to taking viewport scrolling into account?
elementFromPoint and event.client* take viewport scrolling into account, so actually we're making them consistent.
Ah, ok.  Most excellent.
if roc says it's a super-blocker, then it is :) Especially since it's already assigned to him.
Flags: blocking1.9? → blocking1.9+
Comment on attachment 278866 [details] [diff] [review]
fix

r+sr+a1.9=dbaron
Attachment #278866 - Flags: superreview?(dbaron)
Attachment #278866 - Flags: superreview+
Attachment #278866 - Flags: review?(dbaron)
Attachment #278866 - Flags: review+
Attachment #278866 - Flags: approval1.9+
checked in
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Keywords: dev-doc-needed
Duplicate of this bug: 396694
What if anything about this is supposed to be documented?  It appears to simply be about bug fixes to a function that was already new in Fx3 anyway. :)
OK, comment 23 is the thing that needs documenting -- scrolling is taken into account for get(Bounding)ClientRect(s).
I've added text to the articles for both functions mentioning that scrolling is considered.
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.