search

KawaiiBASIC / classilla

Automatically exported from code.google.com/p/classilla
0 stars 0 forks source link

Shift right again [twitter.com] #96

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
On the overflow branch, on Twitter, the favourites star and trash can are
(either) shifted right (or hidden by the border). Ditto for reply/retweet
in the home screen.

This isn't a major deal functionally as the JS doesn't work worth a poop
but this is a fairly irritating layout regression as 9.0 and 9.0.4 both do
this right. Also, this probably affects other sites.

Original issue reported on code.google.com by classi...@floodgap.com on 23 Jan 2010 at 7:24

GoogleCodeExporter commented 9 years ago 
Related to issue 91?

Original comment by classi...@floodgap.com on 23 Jan 2010 at 7:24

GoogleCodeExporter commented 9 years ago 
Made some test cases.

http://www.floodgap.com/software/classilla/bleh/yfrg/

The bug seems to be when overflow is hidden and the element is floating. This
explains why this occurred with the overflow branch (9.1), because we never 
handed
overflow:hidden before. Possible solution is to simply ignore overflow if we are
floating, or to try to use the old behaviour.

Original comment by classi...@floodgap.com on 17 May 2010 at 6:53

GoogleCodeExporter commented 9 years ago 
Notes (pretty much anything that M69355 touched probably should be looked at). 
Seems
to me that we should throw a float check on pretty much all of these. To avoid
getting into conflicts with our somewhat broken nsGfxScrollFrame, I think we'll 
have
such frames treated as _VISIBLE rather than _AUTO.

html/base/src/nsBlockFrame.cpp
  if (NS_FRAME_IS_NOT_COMPLETE(state.mReflowStatus)) {
    //if (NS_STYLE_OVERFLOW_HIDDEN == aReflowState.mStyleDisplay->mOverflow) {
    if (NS_STYLE_OVERFLOW_CLIP == aReflowState.mStyleDisplay->mOverflow) { // bu
g 69355
      state.mReflowStatus = NS_FRAME_COMPLETE;
    }

  if (NS_STYLE_OVERFLOW_CLIP != aReflowState.mStyleDisplay->mOverflow) { // bug 
69355
    for (line_iterator line = begin_lines(), line_end = end_lines();

  if (NS_STYLE_OVERFLOW_CLIP == disp->mOverflow) { // bug 69355
    aRenderingContext.PushState();
    SetOverflowClipRect(aRenderingContext);
  }

  if (NS_STYLE_OVERFLOW_CLIP == disp->mOverflow) { // bug 69355
    PRBool clipState;
    aRenderingContext.PopState(clipState);
  }

html/base/src/nsContainerFrame.cpp
  if (!viewHasTransparentContent) {
    // If we're showing the view but the frame is hidden, then the view is trans
parent
    nsViewVisibility visibility;
    aView->GetVisibility(visibility);
    const nsStyleVisibility* vis;
    ::GetStyleData(aStyleContext, &vis);
    if ((nsViewVisibility_kShow == visibility
         && NS_STYLE_VISIBILITY_HIDDEN == vis->mVisible)
        || (NS_STYLE_OVERFLOW_VISIBLE == display->mOverflow
            && (kidState & NS_FRAME_OUTSIDE_CHILDREN) != 0)) {
      viewHasTransparentContent = PR_TRUE;
    }
  }

  PRBool hasOverflowClip = isBlockLevel && (display->mOverflow ==
NS_STYLE_OVERFLOW_CLIP); // bug 69355

(This is interesting)
  // See if the frame is block-level and has 'overflow' set to 'hidden'. If
  // so, then we need to give it a view so clipping
  // of any child views works correctly. Note that if it's floated it is also
  // block-level, but we can't trust that the style context 'display' value is
  // set correctly
  if ((display->IsBlockLevel() || display->IsFloating()) &&
      //(display->mOverflow == NS_STYLE_OVERFLOW_HIDDEN)) {
      (display->mOverflow == NS_STYLE_OVERFLOW_CLIP)) { // bug 69355

html/base/src/nsFrame.cpp
  //if (display->mOverflow != NS_STYLE_OVERFLOW_HIDDEN) {
  if (NS_STYLE_OVERFLOW_CLIP != display->mOverflow &&
      NS_STYLE_OVERFLOW_HIDDEN != display->mOverflow) { // bug 69355 + backbugs

html/base/src/nsGfxScrollFrame.cpp
There are several things to look at but I think this is most relevant
    // There are two cases to consider
      if (scrolledContentSize.height <= scrollAreaRect.height
          || styleDisplay->mOverflow == NS_STYLE_OVERFLOW_SCROLLBARS_HORIZONTAL
          //|| styleDisplay->mOverflow == NS_STYLE_OVERFLOW_SCROLLBARS_NONE) {
          || styleDisplay->mOverflow == NS_STYLE_OVERFLOW_HIDDEN) { // bug 69355 for
Clecko

thinking we should probably NOT touch this:
    // if the child is wider that the scroll area
    // and we don't have a scrollbar add one.
    if (scrolledContentSize.width > scrollAreaRect.width
        && styleDisplay->mOverflow != NS_STYLE_OVERFLOW_SCROLLBARS_VERTICAL
        //&& styleDisplay->mOverflow != NS_STYLE_OVERFLOW_SCROLLBARS_NONE) { 
        && styleDisplay->mOverflow != NS_STYLE_OVERFLOW_HIDDEN) { // bug 69355 for Clecko

html/style/src/nsCSSFrameConstructor.cpp
  //const nsStyleDisplay* display = styleContext->GetStyleDisplay();
  const nsStyleDisplay *display = (nsStyleDisplay
*)styleContext->GetStyleData(eStyleStruct_Display);
  if (display->mOverflow != NS_STYLE_OVERFLOW_VISIBLE) {
    //aPresContext->SetViewportOverflowOverride(display->mOverflow);
    // tell caller we stole the overflow style from the root element
    return docElement;
  }

  //display = bodyContext->GetStyleDisplay();
  display = (nsStyleDisplay *)bodyContext->GetStyleData(eStyleStruct_Display);
  if (display->mOverflow != NS_STYLE_OVERFLOW_VISIBLE) {
    //aPresContext->SetViewportOverflowOverride(display->mOverflow);
    // tell caller we stole the overflow style from the body element
    return bodyElement;
  }

I do NOT think we should change this, or anything having to do with 
isScrollable:
          if (NS_STYLE_OVERFLOW_HIDDEN == scrolling || NS_STYLE_OVERFLOW_CLIP ==
 scrolling) { // added for bug 69355
            isScrollable = PR_FALSE;
          }

We should look at ::IsScrollable, I don't think this is right for 
_OVERFLOW_HIDDEN

html/table/src/nsTableCellFrame.cpp
      //if ((NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow) || HasPctOverHeight())
 {
      if (NS_STYLE_OVERFLOW_CLIP == disp->mOverflow || // bug 69355
          // XXXldb _HIDDEN should really create a scrollframe,
          // but test here since it doesn't.
          //NS_STYLE_OVERFLOW_SCROLLBARS_NONE == disp->mOverflow ||
          NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow ||
          HasPctOverHeight()) { // bug 221140

html/table/src/nsTableFrame.cpp
This might be a little problematic. haven't thought this one over yet
  PRUint8 overflow = disp->mOverflow; //GetStyleDisplay()->mOverflow;
  PRBool clip = overflow == NS_STYLE_OVERFLOW_HIDDEN ||
                //overflow == NS_STYLE_OVERFLOW_SCROLLBARS_NONE;
                overflow == NS_STYLE_OVERFLOW_CLIP; // backwards from bug 69355
  if (clip) {
    aRenderingContext.PushState();
    SetOverflowClipRect(aRenderingContext);
  }

  // bug 173277
  //if (NS_STYLE_OVERFLOW_HIDDEN != aReflowState.mStyleDisplay->mOverflow) {
  if (NS_STYLE_OVERFLOW_CLIP != aReflowState.mStyleDisplay->mOverflow) { // bug 
69355

html/table/src/nsTableOuterFrame.cpp
(same)
  // If overflow is hidden then set the clip rect so that children
  // don't leak out of us
  //if (NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow) {
  // bug 221140 modified for Clecko
  PRUint8 overflow = disp->mOverflow; //GetStyleDisplay()->mOverflow;
  PRBool clip = overflow == NS_STYLE_OVERFLOW_HIDDEN ||
                //overflow == NS_STYLE_OVERFLOW_SCROLLBARS_NONE;
                overflow == NS_STYLE_OVERFLOW_CLIP; // bug 69355
  if (clip) { // end bug

html/table/src/nsTableRowFrame.cpp
(same)
  // Standards mode background painting removed.  See bug 4510

  const nsStyleDisplay* disp = (const nsStyleDisplay*)
  mStyleContext->GetStyleData(eStyleStruct_Display);
  // bug 221140 modified for Clecko
  PRUint8 overflow = disp->mOverflow; //GetStyleDisplay()->mOverflow;
  PRBool clip = overflow == NS_STYLE_OVERFLOW_HIDDEN ||
                //overflow == NS_STYLE_OVERFLOW_SCROLLBARS_NONE;
                overflow == NS_STYLE_OVERFLOW_CLIP; // bug 69355
  if (clip) { // if (disp && (NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow)) { //

html/table/src/nsTableRowGroupFrame.cpp
(same)
  // Standards mode background painting removed.  See bug 4510

  const nsStyleDisplay* disp = (const nsStyleDisplay*)
    mStyleContext->GetStyleData(eStyleStruct_Display);
  // bug 221140 modified for Clecko
  PRUint8 overflow = disp->mOverflow; // GetStyleDisplay()->mOverflow;
  PRBool clip = overflow == NS_STYLE_OVERFLOW_HIDDEN ||
#ifdef NS_STYLE_MOZ_SCROLLBARS_NONE
                overflow == NS_STYLE_OVERFLOW_MOZ_SCROLLBARS_NONE;
#else
                overflow == NS_STYLE_OVERFLOW_CLIP;
#warning when we define MOZ_SCROLLBARS_NONE this needs to be uncommented
#endif
  if (clip) { //if (disp && (NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow)) { // 

xul/base/src/nsBoxFrame.cpp
(same?)
  // If overflow is hidden then set the clip rect so that children
  // don't leak out of us
  //if (NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow) {
  if (NS_STYLE_OVERFLOW_CLIP == disp->mOverflow) { // bug 69355
    nsMargin im(0,0,0,0);
    GetInset(im);
    r.Deflate(im);
    r.Deflate(border);    
  }

    //if (!hasClipped && NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow) {
    if (!hasClipped && NS_STYLE_OVERFLOW_CLIP == disp->mOverflow) { // bug 69355
        // if we haven't already clipped and we should
        // check to see if the child is in out bounds. If not then
        // we begin clipping.
        nsRect cr(0,0,0,0);
        kid->GetBounds(cr);

    //if (NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow) {
    if (NS_STYLE_OVERFLOW_CLIP == disp->mOverflow) { // bug 69355
      GetDebugMargin(debugMargin);
      PixelMarginToTwips(aPresContext, debugMargin);
      r.Deflate(debugMargin);
    }

         //if (!hasClipped && NS_STYLE_OVERFLOW_HIDDEN == disp->mOverflow) {
         if (!hasClipped && NS_STYLE_OVERFLOW_CLIP == disp->mOverflow) { // bug 
69355
            // if we haven't already clipped and we should
            // check to see if the child is in out bounds. If not then
            // we begin clipping.
            nsRect cr(0,0,0,0);
            kid->GetBounds(cr);

Original comment by classi...@floodgap.com on 27 May 2010 at 4:38

GoogleCodeExporter commented 9 years ago 
I made all those changes and it didn't make a lick of difference. So, in a fit 
of
spite, I went into the rule node's ::ComputeDisplayData and threw in

  // overflow: enum, auto, inherit
// Classilla issue 96
  if (display->mFloats != NS_STYLE_FLOAT_NONE) {
    display->mOverflow = NS_STYLE_OVERFLOW_VISIBLE;
  }
  else

This is probably about the best I can do because I can only examine the display 
node.
This solves yfrog and a lot of other sites, although it drags down 68KMla 
because it
gets the page width wrong (even though it still lays out correctly, it turns on 
a
horizontal scrollbar and the widget switches to the slower painting mode). Just 
for
laughs, let's see what happens if we just limit it to situations where the 
height and
width are completely specified rather than just floating.

Original comment by classi...@floodgap.com on 30 May 2010 at 7:15

GoogleCodeExporter commented 9 years ago 
Note that it only mostly solves Twitter, so it seems we have a second bug 
afoot, but
I'm not too worried about this because the few places where this bug is not 
solved it
appears we don't correctly handle the needed DOM anyway to make those buttons 
work.

Original comment by classi...@floodgap.com on 30 May 2010 at 7:16

GoogleCodeExporter commented 9 years ago 
Already obvious now I can't do this based on height/width because there is no
guarantee position will be set by the time display must be computed, nor is 
there an
obvious way to crawl around laterally in the rule node to get other stuff 
except at
the layout level (which we already know won't work). Wontfixing in favour of 
issue 1,
but I'm keeping the change I made since it seems to be an adequate current fix.

Original comment by classi...@floodgap.com on 30 May 2010 at 7:28

GoogleCodeExporter commented 9 years ago

Original comment by classi...@floodgap.com on 30 May 2010 at 7:53

GoogleCodeExporter commented 9 years ago 
Backed out. Too many pages get hit with the horizontal bar. It's just not worth 
it.

Original comment by classi...@floodgap.com on 3 Jun 2010 at 2:50

GoogleCodeExporter commented 9 years ago 
Never mind, too many sites won't work without it! So now there is a compromise!
(issue 28)

Original comment by classi...@floodgap.com on 3 Jun 2010 at 3:57