luangruo / classilla

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

META: Kludge Tracker #59

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
This documents kludge hacks introduced into Classilla to surmount certain
apparently unsurmountable problems in certain disgusting ways. These
kludges are to be re-examined when code in their purview is touched again
to see if they are still necessary.

BUGS FIXED BY A KLUDGE SHOULD BE DOCUMENTED WITH ISSUE NUMBER HERE ALSO so
that if a better solution is found, their bug can be reopened, the kludge
backed out and a proper fix applied.

To date:

M209694/mm as applied to 1.3 require[ds] a loop catch to prevent it from
going infinite in nsBlockFrame.cpp. This is done in two places. This is
also separately tracked in issue 12 as it has performance impacts.

Issue 54 was fixed in nsDocShell.cpp by forcing ScrollIfAnchor() to return
NS_OK even if the anchor isn't found. The original code returns
NS_ERROR_FAILURE; I'm not sure if this is even desirable in any circumstance.

Original issue reported on code.google.com by classi...@floodgap.com on 15 Sep 2009 at 4:26

GoogleCodeExporter commented 8 years ago
Issue 28 is fixed by using partial invalidation. There has got to be a better 
way.

Original comment by classi...@floodgap.com on 12 Oct 2009 at 8:24

GoogleCodeExporter commented 8 years ago
Issue 48 is currently dealt with using a min-height fixup in the CSS 
declaration,
essentially preventing a min-height: 0 rule from being asserted and delaying 
overflow.

The whole overflow branch problem (issue 66) is currently dealt with using a new
fixup renderer accessible with Cmd-Shift-X. This forces a few low-level blunt
changes, most notably that NS_FRAME_OVERFLOW_HIDDEN is treated as _VISIBLE, in 
the
rule node. Although this crunches a lot of sites, it does allow many others to
finally be at least visible (arstechnica comes to mind), and it does not show 
the
serious artifacting to anywhere near the extent of the real overflow branch 
that is
currently caused by issue 65.

Original comment by classi...@floodgap.com on 20 Oct 2009 at 5:10

GoogleCodeExporter commented 8 years ago
And note that issue 28's kludge was backed out for 9.0.4. Issue 48 can only be 
fixed
fully when issue 66 can land.

Original comment by classi...@floodgap.com on 20 Oct 2009 at 5:11

GoogleCodeExporter commented 8 years ago
I didn't bother filing an issue for this, but between 9.0 and 9.0.4 the CSS 
system
efficiency changes seem to have caused DOM-based style changes to 
intermittently fail
(generally throwing an NS_ERROR_NOT_AVAILABLE in 
nsIDOMCSS2Properties.[property]). A
few minutes in the debugger showed that nsCSSDeclaration::RemoveProperty was 
throwing
this when the property didn't exist "yet" (through the CSS_CHECK macro), which 
we
weren't making as an efficiency. To get around this problem, ::RemoveProperty 
now
masks its own NE_NOT_AVAILABLE and returns NS_OK in that circumstance only, 
since we
can consider the property removed if it never existed in the first place.

Original comment by classi...@floodgap.com on 22 Oct 2009 at 3:46

GoogleCodeExporter commented 8 years ago
The test case for that, btw, was Google's Hide/show options and Show more 
results
from x.y.z options in search.

Original comment by classi...@floodgap.com on 22 Oct 2009 at 3:47

GoogleCodeExporter commented 8 years ago
For Paul P, Blogspot is not correctly starting designMode, which really screws 
up
posting. The problem is that Google/Blogspot doesn't say what its MIME type is, 
which
interferes with creating it (and then execCommand doesn't work etc.) Added

      else if (!mimeCType.Length()) 
      { // KLUDGE. We treat a null string as text/html (example: Blogspot). -- Cameron
        mEditorType = NS_LITERAL_CSTRING("html");
        mimeCType = NS_LITERAL_CSTRING("text/html");
      }         

to SetupEditorOnWindow in nsEditingSession.cpp, as the last condition before it 
sets
status to eEditorErrorCantEditMimeType.

Original comment by classi...@floodgap.com on 23 Oct 2009 at 3:56

GoogleCodeExporter commented 8 years ago
Issue 65 is currently repaired by disabling double buffering if the damage rect 
is
wider or taller than the screen is wide or tall. This generates some light
flickering, although it's a lot better than the completely fractured repaints 
it did
before.

Original comment by classi...@floodgap.com on 19 Jan 2010 at 5:25

GoogleCodeExporter commented 8 years ago
nsEditingSession.cpp in the overflow branch has the change from comment 6 
already
after I hand-merged them.

Comment 4 no longer applies to the overflow branch, as that fix is part of the
changes it introduced.

Original comment by classi...@floodgap.com on 21 Jan 2010 at 6:46

GoogleCodeExporter commented 8 years ago
Issue 93 is repaired by a consensus merge between 1.3.1 and M69355/mm. This 
seems
technically disgusting and I'm not actually sure what it's fixing.

Original comment by classi...@floodgap.com on 26 Jan 2010 at 12:01

GoogleCodeExporter commented 8 years ago
A very complex kludge is currently holding issue 28.

Issue 104 is repaired by simply disabling the .js' trapping of backspace. This 
seems
really disgusting also and I don't know what this disables.

Original comment by classi...@floodgap.com on 12 Feb 2010 at 6:56

GoogleCodeExporter commented 8 years ago
Another complex kludge is mitigating issue 62.

Original comment by classi...@floodgap.com on 25 Feb 2010 at 6:13

GoogleCodeExporter commented 8 years ago
Issue 118 is repaired by some modifications for our ancient DOM glue. When we 
upgrade
DOM, we should open that back up and see if it works without that tweak.

Original comment by classi...@floodgap.com on 17 May 2010 at 11:27

GoogleCodeExporter commented 8 years ago
Issue 122 is repaired by simply limiting the number of times the 
MapNodeInfoInto loop
can execute. Need to review this for next layout iteration. This also applies to
issue 62.

Original comment by classi...@floodgap.com on 23 May 2010 at 8:58

GoogleCodeExporter commented 8 years ago
er, s/Node/Rule/

Original comment by classi...@floodgap.com on 23 May 2010 at 9:01

GoogleCodeExporter commented 8 years ago
Issue 113 is fixed by simply ignoring any existing pluginreg.dat.

Original comment by classi...@floodgap.com on 31 May 2010 at 2:08