Bug 56871 - Clean up: Rename HTMLFrameSetElement::{noresize, frameborder, and frameBorderSet} to conform to the WebKit Code style guidelines
Summary: Clean up: Rename HTMLFrameSetElement::{noresize, frameborder, and frameBorder...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-03-22 15:35 PDT by Daniel Bates
Modified: 2023-12-28 08:27 PST (History)
3 users (show)

See Also:


Attachments
Patch (4.82 KB, patch)
2011-03-22 15:39 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2011-03-22 15:35:27 PDT
We should prefix the instance variables  HTMLFrameSetElement::{noresize, frameborder, and frameBorderSet} with "m_" to conform to the WebKit Code style guidelines.
Comment 1 Daniel Bates 2011-03-22 15:39:30 PDT
Created attachment 86525 [details]
Patch

Renamed HTMLFrameSetElement::frameBorderSet to HTMLFrameSetElement::m_frameborderSet since frameborder to match the name of the HTML attribute that corresponds to this instance variable.
Comment 2 Daniel Bates 2011-03-23 10:58:57 PDT
Committed r81787: <http://trac.webkit.org/changeset/81787>
Comment 3 Darin Adler 2011-03-23 11:00:01 PDT
Nice to add an m and underscore but also would be great to give these better names.
Comment 4 Daniel Bates 2011-03-23 11:26:46 PDT
(In reply to comment #3)
> Nice to add an m and underscore but also would be great to give these better names.

I agree. I was not very satisfied with these names or the names, but I couldn't think of any better names at the time. I am open to suggestions. Let me know if you would like me to roll out changeset 81787 <http://trac.webkit.org/changeset/81787> and/or feel free to roll it out. 

For frameBorderSet, I was originally thinking m_hasFrameborderSet, m_isFrameborderSet, or m_wasFrameborderAttributeSpecified. Similarly, I thought to rename frameborder to m_hasFrameBorder to be consistent with the getter of the same name. Note, I am also unhappy about the names of the existing instance variables m_borderSet, m_border, or m_borderColorSet. We should come up with some convention so that we can rename m_borderSet, m_border, m_borderColorSet, m_frameborder,  m_frameborderSet, and m_noresize.

At the time I wrote this patch, I chose to defer more descriptive renames and make frameborder, frameBorderSet, and noresize more consistent to the names of the existing instance variables m_border, m_borderSet because I was unsure of the naming convention I would use for renaming m_borderSet, m_border, and m_borderColorSet and was worried that committing subsequent patches that would change the convention would be noisy.
Comment 5 Daniel Bates 2011-03-23 11:28:28 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Nice to add an m and underscore but also would be great to give these better names.
> 
> I agree. I was not very satisfied with these names or the names, but I couldn't think of any better names 

This should read:

I agree. I was not very satisfied with these names, but I couldn't think of a better naming convention at the time.
Comment 6 Daniel Bates 2011-04-07 12:43:12 PDT
Comment on attachment 86525 [details]
Patch

Clearing review flag and marking as obsolete so as to remove this patch from the commit queue. We should still think about a better naming convention for these variables.
Comment 7 Anne van Kesteren 2023-12-28 08:26:40 PST
These seem okay as-is, especially given it's a legacy element. Might as well name the members in a way that matches the specification.
Comment 8 Radar WebKit Bug Importer 2023-12-28 08:27:14 PST
<rdar://problem/120248244>